fix(WireGuardTunnel): adjust 'addresses' validation pre-condition timing #902

The previous fix did not address the outer nested model field validation, only the inner validation which rendered the fix unusable for many use cases. This commits introduces a new injection point to update() that allows us to apply pre-conditions before model validation, thefor allow us to negate addresses before validation if an iface assignment exists.

Author: jaredhendrickson13

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc

@@ -1829,6 +1829,13 @@ class Model {
         $this->apply();
     }
 
+    /**
+     * Initializes the 'pre_validate_update()' method. This method is intended to be overridden by a child model class
+     * and is called immediately before validating update actions. This is useful for conditions that need to be
+     * considered before validation occurs, such as scrubbing certain fields.
+     */
+    protected function pre_validate_update(): void {}
+
     /**
      * Initializes the default 'pre_apply_update' method. This method is intended to be overridden by a child model class and
      * is called immediately before the 'apply' method for update actions only. This method runs regardless of whether
@@ -2352,6 +2359,9 @@ class Model {
             $this->remove_array_changes();
         }
 
+        # Run the pre-validate method to allow for any adjustments to the object before validation occurs
+        $this->pre_validate_update();
+
         # Ensure all object Fields and validations succeed for proceeding.
         if ($this->validate(requires_id: true)) {
             # When dry_run is enabled, skip the actual write/apply phase but still report the would-be result

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardTunnel.inc

@@ -145,18 +145,6 @@ class WireGuardTunnel extends Model {
         return wg_is_key_clamped($privatekey) ? $privatekey : wg_clamp_key($privatekey);
     }
 
-    /**
-     * Extends the default _update method to ensure addresses are removed if the tunnel has an interface assignment
-     */
-    public function _update(): void {
-        # Remove any existing addresses if this tunnel has an existing interface assignment
-        if (NetworkInterface::query(if: $this->name->value)->exists()) {
-            $this->addresses->value = [];
-        }
-
-        parent::_update();
-    }
-
     /**
      * Obtains the next available WireGuard tunnel interface name.
      * @return string The next available WireGuard tunnel interface name (i.e. tun_wg0)
@@ -165,6 +153,18 @@ class WireGuardTunnel extends Model {
         return next_wg_if();
     }
 
+    /**
+     * Adds pre-validation logic to scrub addresses from this WireGuardTunnel if it has an existing interface
+     * assignment.
+     */
+    protected function pre_validate_update(): void {
+        # If this tunnel is assigned to an existing pfSense interface, scrub any addresses from the `addresses` field
+        # since the addresses for this tunnel will be derived from the assigned interface's configured IPs instead.
+        if (NetworkInterface::query(if: $this->name->value)->exists()) {
+            $this->addresses->value = [];
+        }
+    }
+
     /**
      * Serializes changes to this tunnel before applying.
      */

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsWireGuardTunnelTestCase.inc

@@ -116,6 +116,60 @@ class APIModelsWireGuardTunnelTestCase extends TestCase {
         $tun_wg0->delete(apply: true);
     }
 
+    /**
+     * Checks that the `addresses` field is removed before an update when the tunnel has an existing interface
+     * assignment, since addresses are derived from the assigned interface's configured IPs instead.
+     */
+    public function test_addresses_removed_before_update_when_interface_is_assigned(): void {
+        # Create a WireGuardTunnel to test with
+        $tunnel = new WireGuardTunnel(privatekey: 'KG0BA4UyPilHH5qnXCfr6Lw8ynecOPor88tljLy3AHk=', async: false);
+        $tunnel->create(apply: true);
+
+        # Assign a NetworkInterface for this tunnel
+        $if = new NetworkInterface(
+            if: $tunnel->name->value,
+            descr: 'WGTEST',
+            enable: true,
+            typev4: 'none',
+            typev6: 'none',
+            async: false,
+        );
+        $if->create();
+
+        # Update the tunnel with addresses while it has an interface assignment
+        # pre_validate_update() should scrub the addresses before validation runs
+        $tunnel->from_representation(addresses: [['address' => '172.20.99.1', 'mask' => 30]]);
+        $tunnel->update(apply: false);
+
+        # Ensure the addresses were removed before the update was processed
+        $this->assert_is_empty($tunnel->addresses->value);
+
+        # Delete the interface and the tunnel
+        $if->delete(apply: true);
+        $tunnel->delete(apply: true);
+    }
+
+    /**
+     * Checks that the `addresses` field is left intact before an update when the tunnel has no existing
+     * interface assignment.
+     */
+    public function test_addresses_retained_on_update_when_no_interface_assigned(): void {
+        # Create a WireGuardTunnel to test with
+        $tunnel = new WireGuardTunnel(privatekey: 'KG0BA4UyPilHH5qnXCfr6Lw8ynecOPor88tljLy3AHk=', async: false);
+        $tunnel->create(apply: true);
+
+        # Update the tunnel with addresses without an interface assignment
+        # pre_validate_update() should leave the addresses intact
+        $tunnel->from_representation(addresses: [['address' => '172.20.99.1', 'mask' => 30]]);
+        $tunnel->update(apply: true);
+
+        # Ensure the addresses were retained during the update
+        $this->assert_is_not_empty($tunnel->addresses->value);
+
+        # Delete the tunnel
+        $tunnel->delete(apply: true);
+    }
+
     /**
      * Checks that the tunnel is properly configured on the backend after creating, updating and deleting
      */