From 1fc5a6335e9e3f8a5e957fbfdc2e9876dcd4736f Mon Sep 17 00:00:00 2001 From: Stephan de Wit Date: Thu, 12 Dec 2024 11:17:39 +0100 Subject: [PATCH] system: refactor system status mechanism, introduce persistent notifications Also introduces better sorting with a separate priority value as well as a refactored frontend. Includes some fixes for missing translations as well. To test a banner such as "the system is booting": flock -n -o /var/run/booting cat --- .../OPNsense/Core/Api/SystemController.php | 41 +++- .../OPNsense/System/AbstractStatus.php | 36 ++- .../System/Status/CrashReporterStatus.php | 11 +- .../OPNsense/System/Status/FirewallStatus.php | 9 +- .../System/Status/LiveMediaStatus.php | 8 +- .../System/Status/SystemBootingStatus.php | 8 +- .../library/OPNsense/System/SystemStatus.php | 7 +- .../OPNsense/System/SystemStatusCode.php | 45 ++++ .../mvc/app/views/layouts/default.volt | 23 +- src/opnsense/www/js/opnsense_status.js | 208 ++++++++++++------ src/www/fbegin.inc | 5 + src/www/head.inc | 17 +- 12 files changed, 280 insertions(+), 138 deletions(-) create mode 100644 src/opnsense/mvc/app/library/OPNsense/System/SystemStatusCode.php diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/SystemController.php b/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/SystemController.php index 935e8bd13..f4dd4d0b6 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/SystemController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/SystemController.php @@ -1,7 +1,7 @@ configdRun('system status')), true); if ($statuses) { - $order = [-1 => 'Error', 0 => 'Warning', 1 => 'Notice', 2 => 'OK']; - + $order = SystemStatusCode::toValueNameArray(); $acl = new ACL(); + foreach ($statuses as $subsystem => $status) { $statuses[$subsystem]['status'] = $order[$status['statusCode']]; if (!empty($status['logLocation'])) { if (!$acl->isPageAccessible($this->getUserName(), $status['logLocation'])) { unset($statuses[$subsystem]); + continue; } - } else { - /* XXX exiting loop causing global endpoint failure */ - return $response; } } - /* Sort on the highest error level after the ACL check */ + /* Sort on the highest notification (non-persistent) error level after the ACL check */ $statusCodes = array_map(function ($v) { - return $v['statusCode']; + return $v['persistent'] ? SystemStatusCode::OK : $v['statusCode']; }, array_values($statuses)); sort($statusCodes); - $statuses['System'] = [ - 'status' => $order[$statusCodes[0] ?? 2] + + $response['metadata'] = [ + /* 'system' represents the status of the top notification after sorting */ + 'system' => [ + 'status' => $order[$statusCodes[0] ?? 2], + 'message' => gettext('No pending messages'), + 'title' => gettext('System'), + ], + 'translations' => [ + 'dialogTitle' => gettext('System Status'), + 'dialogCloseButton' => gettext('Close') + ] ]; + // sort on status code and priority where priority is the tie breaker + uasort($statuses, function ($a, $b) { + if ($a['statusCode'] === $b['statusCode']) { + return $a['priority'] <=> $b['priority']; + } + return $a['statusCode'] <=> $b['statusCode']; + }); + foreach ($statuses as &$status) { if (!empty($status['timestamp'])) { $age = time() - $status['timestamp']; @@ -150,7 +168,8 @@ class SystemController extends ApiControllerBase } } - $response = $statuses; + $response['subsystems'] = $statuses; + unset($response['status']); } return $response; diff --git a/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php index 3a4a4a90b..d91a38e45 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php @@ -1,7 +1,7 @@ internalPriority; + } + + public function getPersistent() + { + return $this->internalPersistent; + } + + public function getTitle() + { + return $this->internalTitle; + } public function getStatus() { return $this->internalStatus; } - public function getMessage($verbose = false) + public function getMessage() { - return $this->internalMessage; + return $this->internalMessage ?? gettext('No problems were detected.'); } public function getLogLocation() diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php index 69a7a363c..47d554d0d 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php @@ -1,7 +1,7 @@ internalPriority = 10; + $this->internalTitle = gettext('Crash Reporter'); $this->internalLogLocation = '/crash_reporter.php'; $src_errors = count($src_logs) > 0; @@ -61,10 +64,10 @@ class CrashReporterStatus extends AbstractStatus if ($php_errors || $src_errors) { $this->internalMessage = gettext('An issue was detected and can be reviewed using the firmware crash reporter.'); if ($php_errors) { - $this->internalStatus = Config::getInstance()->object()->system->deployment != 'development' ? static::STATUS_ERROR : static::STATUS_NOTICE; + $this->internalStatus = Config::getInstance()->object()->system->deployment != 'development' ? SystemStatusCode::ERROR : SystemStatusCode::NOTICE; } - if ($src_errors && $this->internalStatus != static::STATUS_ERROR) { - $this->internalStatus = static::STATUS_WARNING; + if ($src_errors && $this->internalStatus != SystemStatusCode::ERROR) { + $this->internalStatus = SystemStatusCode::WARNING; } } } diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php index 58742894b..0fd4f6007 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php @@ -1,7 +1,7 @@ internalPriority = 20; + $this->internalTitle = gettext('Firewall'); $this->internalLogLocation = '/ui/diagnostics/log/core/firewall'; if (file_exists($this->rules_error)) { - $this->internalMessage = file_get_contents($this->rules_error); - $this->internalStatus = static::STATUS_ERROR; + $this->internalMessage = file_get_contents($this->rules_error); /* XXX */ + $this->internalStatus = SystemStatusCode::ERROR; $info = stat($this->rules_error); if (!empty($info['mtime'])) { $this->internalTimestamp = $info['mtime']; diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php index 587cfdd6e..7107286e6 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php @@ -29,14 +29,16 @@ namespace OPNsense\System\Status; use OPNsense\System\AbstractStatus; +use OPNsense\System\SystemStatusCode; use OPNsense\Core\Config; class LiveMediaStatus extends AbstractStatus { public function __construct() { - /* XXX historically tied to the dashboard but only given because controller will not allow an omission */ - $this->internalLogLocation = '/ui/core/dashboard'; + $this->internalPriority = 2; + $this->internalPersistent = true; + $this->internalTitle = gettext('Live Media'); /* * Despite unionfs underneath, / is still not writeable, @@ -54,7 +56,7 @@ class LiveMediaStatus extends AbstractStatus return; } - $this->internalStatus = static::STATUS_WARNING; + $this->internalStatus = SystemStatusCode::NOTICE; $this->internalMessage = gettext('You are currently running in live media mode. A reboot will reset the configuration.'); if (empty(Config::getInstance()->object()->system->ssh->noauto)) { exec('/bin/pgrep -anx sshd', $output, $retval); /* XXX portability shortcut */ diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php index f10064e53..9db3e3445 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php @@ -29,20 +29,22 @@ namespace OPNsense\System\Status; use OPNsense\System\AbstractStatus; +use OPNsense\System\SystemStatusCode; class SystemBootingStatus extends AbstractStatus { public function __construct() { - /* XXX historically tied to the dashboard but only given because controller will not allow an omission */ - $this->internalLogLocation = '/ui/core/dashboard'; + $this->internalPriority = 1; + $this->internalPersistent = true; + $this->internalTitle = gettext('System Booting'); /* XXX boot detection from final class product in config.inc */ $fp = fopen('/var/run/booting', 'a+e'); if ($fp) { if (!flock($fp, LOCK_SH | LOCK_NB)) { $this->internalMessage = gettext('The system is currently booting. Not all services have been started yet.'); - $this->internalStatus = static::STATUS_WARNING; + $this->internalStatus = SystemStatusCode::WARNING; } fclose($fp); } diff --git a/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php index 840230458..9457b5b77 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php @@ -1,7 +1,7 @@ getShortName()); + $shortName = strtolower(str_replace('Status', '', $reflect->getShortName())); $this->objectMap[$shortName] = $obj; if ($shortName == 'System') { @@ -72,10 +72,13 @@ class SystemStatus } $result[$shortName] = [ + 'title' => $obj->getTitle(), 'statusCode' => $obj->getStatus(), 'message' => $obj->getMessage(), 'logLocation' => $obj->getLogLocation(), 'timestamp' => $obj->getTimestamp(), + 'persistent' => $obj->getPersistent(), + 'priority' => $obj->getPriority(), ]; } diff --git a/src/opnsense/mvc/app/library/OPNsense/System/SystemStatusCode.php b/src/opnsense/mvc/app/library/OPNsense/System/SystemStatusCode.php new file mode 100644 index 000000000..dac190f6c --- /dev/null +++ b/src/opnsense/mvc/app/library/OPNsense/System/SystemStatusCode.php @@ -0,0 +1,45 @@ +value] = $case->name; + } + return $result; + } +} diff --git a/src/opnsense/mvc/app/views/layouts/default.volt b/src/opnsense/mvc/app/views/layouts/default.volt index 80a8881c8..789fa8dc8 100644 --- a/src/opnsense/mvc/app/views/layouts/default.volt +++ b/src/opnsense/mvc/app/views/layouts/default.volt @@ -96,22 +96,9 @@ initFormAdvancedUI(); addMultiSelectClearUI(); - // Create status dialog instance - let dialog = new BootstrapDialog({ - title: '{{ lang._('System Status')}}', - buttons: [{ - label: '{{ lang._('Close') }}', - action: function(dialogRef) { - dialogRef.close(); - } - }], - }); - + // artificial delay for UX reasons setTimeout(function () { - updateSystemStatus().then((data) => { - let status = parseStatus(data); - registerStatusDelegate(dialog, status); - }); + updateSystemStatus(); }, 500); // Register collapsible table headers @@ -279,6 +266,12 @@ + + +
+ +
+
diff --git a/src/opnsense/www/js/opnsense_status.js b/src/opnsense/www/js/opnsense_status.js index 903dadb16..ac1054185 100644 --- a/src/opnsense/www/js/opnsense_status.js +++ b/src/opnsense/www/js/opnsense_status.js @@ -1,5 +1,5 @@ /** - * Copyright (C) 2022 Deciso B.V. + * Copyright (C) 2022-2024 Deciso B.V. * * All rights reserved. * @@ -25,53 +25,63 @@ * POSSIBILITY OF SUCH DAMAGE. */ -function updateStatusDialog(dialog, status, subjectRef = null) { - let $ret = $('

 System

No pending messages.

'); +function updateStatusDialog(dialog, status) { + let $ret = $(` +
+ +

+ + +   + System +

+

No pending messages.

+ +
+ `); + let $message = $( '
' + '
' + '
' ); - for (let subject in status.data) { - if (subject === 'System') { - continue; - } - let statusObject = status.data[subject]; - if (status.data[subject].status == "OK") { - continue; - } - + for (let [shortname, subject] of Object.entries(status)) { $message.find('a').last().addClass('__mb'); - let formattedSubject = subject.replace(/([A-Z])/g, ' $1').trim(); - if (status.data[subject].age != undefined) { - formattedSubject += ' (' + status.data[subject].age + ')'; + let formattedSubject = subject.title; + if (subject.age != undefined) { + formattedSubject += ' (' + subject.age + ')'; } - let listItem = '' + - '

 ' + formattedSubject + - '

' + - '

' + statusObject.message + '

'; - let referral = statusObject.logLocation; + let ref = subject.logLocation != null ? `href="${subject.logLocation}"` : ''; + let $closeBtn = ` + + `; + let $listItem = $(` + +

+ +   + ${formattedSubject} + ${subject.persistent ? '' : $closeBtn} +

+

${subject.message}

+ + `) - $message.find('#opn-status-list').append(listItem); + $message.find('#opn-status-list').append($listItem); - $message.find('#dismiss-' + subject).on('click', function (e) { + $message.find('#dismiss-' + shortname).on('click', function (e) { e.preventDefault(); $.ajax('/api/core/system/dismissStatus', { type: 'post', - data: {'subject': subject}, + data: {'subject': shortname}, dialogRef: dialog, - subjectRef: subject, success: function() { - updateSystemStatus().then((data) => { - let newStatus = parseStatus(data); - let $newMessage = updateStatusDialog(this.dialogRef, newStatus, this.subjectRef); - this.dialogRef.setMessage($newMessage); - $('#system_status').attr("class", newStatus.data['System'].icon); - registerStatusDelegate(this.dialogRef, newStatus); - }); + updateSystemStatus(this.dialogRef); } }); }); @@ -81,49 +91,107 @@ function updateStatusDialog(dialog, status, subjectRef = null) { return $ret; } -function parseStatus(data) { - let status = {}; - let severity = BootstrapDialog.TYPE_PRIMARY; - $.each(data, function(subject, statusObject) { - switch (statusObject.status) { - case "Error": - statusObject.icon = 'fa fa-circle text-danger' - if (subject != 'System') break; - severity = BootstrapDialog.TYPE_DANGER; - break; - case "Warning": - statusObject.icon = 'fa fa-circle text-warning'; - if (subject != 'System') break; - severity = BootstrapDialog.TYPE_WARNING; - break; - case "Notice": - statusObject.icon = 'fa fa-circle text-info'; - if (subject != 'System') break; - severity = BootstrapDialog.TYPE_INFO; - break; - default: - statusObject.icon = 'fa fa-circle text-muted'; - if (subject != 'System') break; - break; - } - $('#system_status').removeClass().addClass(statusObject.icon); - }); - status.severity = severity; - status.data = data; - - return status; +function parseStatusIcon(subject) { + switch (subject.status) { + case "ERROR": + subject.icon = 'fa fa-circle text-danger'; + subject.banner = 'alert-danger'; + subject.severity = BootstrapDialog.TYPE_DANGER; + break; + case "WARNING": + subject.icon = 'fa fa-circle text-warning'; + subject.banner ='alert-warning'; + subject.severity = BootstrapDialog.TYPE_WARNING; + break; + case "NOTICE": + subject.icon = 'fa fa-circle text-info'; + subject.banner = 'alert-info'; + subject.severity = BootstrapDialog.TYPE_INFO; + break; + default: + subject.icon = 'fa fa-circle text-muted'; + subject.banner = 'alert-info'; + subject.severity = BootstrapDialog.TYPE_PRIMARY; + break; + } } -function registerStatusDelegate(dialog, status) { - $("#system_status").click(function() { - dialog.setMessage(function(dialogRef) { - let $message = updateStatusDialog(dialogRef, status); - return $message; +function fetchSystemStatus() { + return new Promise((resolve, reject) => { + ajaxGet('/api/core/system/status', {}, function (data) { + resolve(data); }); - dialog.open(); }); } -function updateSystemStatus() { - return $.ajax('/api/core/system/status', { type: 'get', dataType: 'json' }); +function parseStatus(data) { + let system = data.metadata.system; + + // handle initial page load status icon + parseStatusIcon(system); + $('#system_status').removeClass().addClass(system.icon); + + let notifications = {}; + let bannerMessages = {}; + for (let [shortname, subject] of Object.entries(data.subsystems)) { + parseStatusIcon(subject); + + if (subject.status == "OK") + continue; + + if (subject.persistent) { + bannerMessages[shortname] = subject; + } else { + notifications[shortname] = subject; + } + } + + return { + 'banners': bannerMessages, + 'notifications': notifications + }; +} + +function updateSystemStatus(dialog = null) { + fetchSystemStatus().then((data) => { + let status = parseStatus(data); // will also update status icon + + if (dialog != null) { + dialog.setMessage(function(dialogRef) { + return updateStatusDialog(dialogRef, status.notifications); + }) + } + + if (!$.isEmptyObject(status.banners)) { + let banner = Object.values(status.banners)[0]; + $('#notification-banner').addClass(banner.banner).show().html(banner.message); + } + }); + + $("#system_status").click(function() { + fetchSystemStatus().then((data) => { + let translations = data.metadata.translations; + let status = parseStatus(data); + + dialog = new BootstrapDialog({ + title: translations.dialogTitle, + buttons: [{ + id: 'close', + label: translations.dialogCloseButton, + action: function(dialogRef) { + dialogRef.close(); + } + }], + }); + + dialog.setMessage(function(dialogRef) { + // intentionally do banners first, as these should always show on top + // in both cases normal backend sorting applies + return updateStatusDialog(dialogRef, {...status.banners, ...status.notifications}); + }) + + dialog.open(); + }); + }); + } diff --git a/src/www/fbegin.inc b/src/www/fbegin.inc index 7f6072247..f99e46644 100644 --- a/src/www/fbegin.inc +++ b/src/www/fbegin.inc @@ -198,3 +198,8 @@ $aclObj = new \OPNsense\Core\ACL(); + + +
+ +
diff --git a/src/www/head.inc b/src/www/head.inc index 4c4fa5460..ae0309e14 100644 --- a/src/www/head.inc +++ b/src/www/head.inc @@ -183,22 +183,9 @@ $pagetitle .= html_safe(sprintf(' | %s.%s', $config['system']['hostname'], $conf $('html,aside').scrollTop(($(this).offset().top - navbar_center)); }); - // Create status dialog instance - let dialog = new BootstrapDialog({ - title: "", - buttons: [{ - label: "", - action: function(dialogRef) { - dialogRef.close(); - } - }], - }); - + // artifical delay for UX reasons setTimeout(function () { - updateSystemStatus().then((data) => { - let status = parseStatus(data); - registerStatusDelegate(dialog, status); - }); + updateSystemStatus(); }, 500); // hook in live menu search