Skip to content

Gateway Groups: convert to MVC#10205

Open
swhite2 wants to merge 18 commits intomasterfrom
gwgroups_mvc
Open

Gateway Groups: convert to MVC#10205
swhite2 wants to merge 18 commits intomasterfrom
gwgroups_mvc

Conversation

@swhite2
Copy link
Copy Markdown
Member

@swhite2 swhite2 commented Apr 24, 2026

Important notices

Before you submit a pull request, we ask you kindly to acknowledge the following:

If AI was used, please disclose:

  • Model used:
  • Extent of AI involvement:

Describe the problem

Gateway Groups are part of the legacy code base


Describe the proposed solution

Code performs an inline migration and updates only some callers that directly depended on the old configuration bits. Some convenience functions have been added to GatewayGroups.php to allow for easy configuration access and to satisfy some of the caller demands.

In addition, this code Adjusts the gateway_watcher to now trigger based on Gateway Group trigger level as well, limiting the amount of times alarms/alerts are being generated (and thus potentially unnecessary fw reconfigurations).


Related issue

#9944

@swhite2 swhite2 self-assigned this Apr 24, 2026
Comment thread src/opnsense/scripts/monit/gateway_alert.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Routing/Api/SettingsController.php Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.xml Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/FieldTypes/GatewayGroupItemField.php Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/FieldTypes/GatewayGroupItemField.php Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.php Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.php Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.php
Comment thread src/opnsense/scripts/routes/gateways.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Routing/Api/GroupSettingsController.php Outdated
@swhite2 swhite2 marked this pull request as ready for review April 30, 2026 14:13
Comment thread src/opnsense/mvc/app/views/OPNsense/Routing/groups.volt
*/
class GatewayController extends ApiControllerBase
{
/* XXX No callers in core. Deprecate for 26.7? */
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.

Review note: there don't seem to be any active consumers, also the endpoint is misaligned with the rest of the gateways namespace.

Added the comment for discussion purposes, we can also ditch it or do nothing ;)

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.

More or less superseded by /api/routing/settings/search_gateway it seems, but now killed due to the change in gateway_status.php.

We should either keep the script as is or make sure callers are refactored to cope with the change.

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.

I'll interpret this as keep it around. It wasn't killed, but the output changed indeed which should be addressed with 2b5ef1f

Comment on lines +98 to +109
$this_gw_status = $gateways_status[$gateway['name']] ?? [];
$gateways[$idx]['status'] = $this_gw_status['status_translated'] ?? gettext('Pending');
foreach (['delay', 'stddev', 'loss'] as $status_kw) {
$gateways[$idx][$status_kw] = $i !== false ? $gateways_status[$i][$status_kw] : '~';
$gateways[$idx][$status_kw] = $this_gw_status[$status_kw] ?? '~';
}
$gateways[$idx]['label_class'] = 'fa fa-plug text-default';
if ($i !== false) {
if (str_contains($gateways_status[$i]['status'], 'down')) {
if (!empty($this_gw_status)) {
if (str_contains($this_gw_status['status'], 'down')) {
$gateways[$idx]['label_class'] = 'fa fa-plug text-danger';
} elseif (str_contains($gateways_status[$i]['status'], 'loss') || str_contains($gateways_status[$i]['status'], 'delay')) {
} elseif (str_contains($this_gw_status['status'], 'loss') || str_contains($this_gw_status['status'], 'delay')) {
$gateways[$idx]['label_class'] = 'fa fa-plug text-warning';
} elseif (str_contains($gateways_status[$i]['status'], 'none')) {
} elseif (str_contains($this_gw_status['status'], 'none')) {
Copy link
Copy Markdown
Member Author

@swhite2 swhite2 May 1, 2026

Choose a reason for hiding this comment

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

Review note: backend script gateway_status.php now indexes by gateway name, which simplifies the logic above a bit.

*/
class GatewayController extends ApiControllerBase
{
/* XXX No callers in core. Deprecate for 26.7? */
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.

More or less superseded by /api/routing/settings/search_gateway it seems, but now killed due to the change in gateway_status.php.

We should either keep the script as is or make sure callers are refactored to cope with the change.

Comment thread src/opnsense/mvc/app/controllers/OPNsense/Routing/Api/GroupSettingsController.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Routing/Api/GroupSettingsController.php Outdated
{
$gateways_status = json_decode((new Backend())->configdRun('interface gateways status'), true) ?? [];
$config = (new GatewayGroups())->getGroupsConfig();
$result = $this->searchBase('gateway_group');
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 can probably use a $filter_funct to merge $gateways_status with the usual content.

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.

$record in $filter_func is of type BaseField, unlike searchRecordSetBase which makes property assignment a bit more difficult here as far as I know

Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.xml Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.xml Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.xml Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Routing/GatewayGroups.xml Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants