From fdc8a8fd2c3c59ddba1ee3be3285378e9e742a48 Mon Sep 17 00:00:00 2001
From: Stephan de Wit <stephan.de.wit@deciso.com>
Date: Wed, 14 Feb 2024 14:26:50 +0100
Subject: [PATCH] gateways: fix migration issue causing gateways to be skipped

Properties should be copied 1-to-1 before we apply the required
defaults if necessary. In the previous situation this caused
required properties to be set to an empty value after the default
value had already been written to it. In the failure case we
attempt to be a bit more explicit and refer to the crash reporter.

While here, the master branch has dropped the Phalcon Messages class,
so switch to count() since this seems to inherit array type
and is therefore backwards compatible:

$msgs = new \Phalcon\Messages\Messages();
$count = count($msgs);
// $count == 0

See https://github.com/opnsense/core/issues/6389
---
 .../OPNsense/Routing/Migrations/M1_0_0.php    | 46 +++++++++++--------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/opnsense/mvc/app/models/OPNsense/Routing/Migrations/M1_0_0.php b/src/opnsense/mvc/app/models/OPNsense/Routing/Migrations/M1_0_0.php
index 0b93a5357..c87fbcea4 100644
--- a/src/opnsense/mvc/app/models/OPNsense/Routing/Migrations/M1_0_0.php
+++ b/src/opnsense/mvc/app/models/OPNsense/Routing/Migrations/M1_0_0.php
@@ -44,28 +44,12 @@ class M1_0_0 extends BaseModelMigration
         $config = Config::getInstance()->object();
 
         // create logger to save possible consistency issues to
-        $logger =  new Syslog('config', null, LOG_LOCAL2);
+        $logger = new Syslog('config', null, LOG_LOCAL2);
 
         if (!empty($config->gateways) && count($config->gateways->children()) > 0) {
             foreach ($config->gateways->gateway_item as $gateway) {
                 $node = $model->gateway_item->Add();
 
-                // special handling of implied booleans
-                $node->defaultgw = !empty((string)$gateway->defaultgw) ? '1' : '0';
-                $node->disabled = !empty((string)$gateway->disabled) ? '1' : '0';
-                $node->fargw = !empty((string)$gateway->fargw) ? '1' : '0';
-                $node->force_down = !empty((string)$gateway->force_down) ? '1' : '0';
-                $node->monitor_disable = !empty((string)$gateway->monitor_disable) ? '1' : '0';
-                $node->monitor_noroute = !empty((string)$gateway->monitor_noroute) ? '1' : '0';
-
-                if (empty((string)$gateway->priority)) {
-                    $node->priority = '255';
-                }
-
-                if (empty((string)$gateway->ipprotocol)) {
-                    $node->ipprotocol = 'inet';
-                }
-
                 // migrate set nodes
                 $node_properties = iterator_to_array($node->iterateItems());
                 foreach ($gateway as $key => $value) {
@@ -83,6 +67,26 @@ class M1_0_0 extends BaseModelMigration
                     $node->$key = (string)$value;
                 }
 
+                // special handling of implied booleans
+                $node->defaultgw = !empty((string)$gateway->defaultgw) ? '1' : '0';
+                $node->disabled = !empty((string)$gateway->disabled) ? '1' : '0';
+                $node->fargw = !empty((string)$gateway->fargw) ? '1' : '0';
+                $node->force_down = !empty((string)$gateway->force_down) ? '1' : '0';
+                $node->monitor_disable = !empty((string)$gateway->monitor_disable) ? '1' : '0';
+                $node->monitor_noroute = !empty((string)$gateway->monitor_noroute) ? '1' : '0';
+
+                if (empty((string)$gateway->priority)) {
+                    $node->priority = '255';
+                }
+
+                if (empty((string)$gateway->ipprotocol)) {
+                    $node->ipprotocol = 'inet';
+                }
+
+                if (empty((string)$gateway->weight)) {
+                    $node->weight = '1';
+                }
+
                 $model->gateway_item->calculateCurrent($node);
                 // increase time period if old model had it set too low
                 $min_time_period = 2 * (
@@ -91,8 +95,12 @@ class M1_0_0 extends BaseModelMigration
                 if ((string)$node->current_time_period < $min_time_period) {
                     $node->time_period = $min_time_period;
                 }
-                if ($model->performValidation()->count() > 0) {
-                    $logger->error(sprintf("Migration skipped gateway %s (%s)", $gateway->name, $gateway->gateway));
+                $result = $model->performValidation();
+                if (count($result) > 0) {
+                    // save details of validation error
+                    error_log(print_r($result, true));
+                    $logger->error(sprintf("Migration skipped gateway %s (%s). See crash reporter for details",
+                        $gateway->name, $gateway->gateway));
                     $model->gateway_item->del($node->getAttribute('uuid'));
                 }
             }