From 5fa56c8bbabc580a1f0ffcf380822ec19669422d Mon Sep 17 00:00:00 2001
From: themylogin <themylogin@gmail.com>
Date: Fri, 24 Jan 2025 17:29:27 +0100
Subject: [PATCH] Delete DB row for invalid tunables

---
 .../middlewared/plugins/tunables.py           | 62 +++++++++++--------
 tests/api2/test_tunables.py                   | 37 +++++++++++
 2 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/src/middlewared/middlewared/plugins/tunables.py b/src/middlewared/middlewared/plugins/tunables.py
index 5f26240a7428a..c725485f48657 100644
--- a/src/middlewared/middlewared/plugins/tunables.py
+++ b/src/middlewared/middlewared/plugins/tunables.py
@@ -162,17 +162,21 @@ async def do_create(self, job, data):
             'datastore.insert', self._config.datastore, data, {'prefix': self._config.datastore_prefix}
         )
 
-        if data['type'] == 'SYSCTL':
-            if data['enabled']:
-                await self.middleware.call('etc.generate', 'sysctl')
-                await self.middleware.call('tunable.set_sysctl', data['var'], data['value'])
-        elif data['type'] == 'ZFS':
-            if data['enabled']:
-                await self.middleware.call('tunable.set_zfs_parameter', data['var'], data['value'])
-                if update_initramfs:
-                    await self.middleware.call('boot.update_initramfs')
-        else:
-            await self.handle_tunable_change(data)
+        try:
+            if data['type'] == 'SYSCTL':
+                if data['enabled']:
+                    await self.middleware.call('etc.generate', 'sysctl')
+                    await self.middleware.call('tunable.set_sysctl', data['var'], data['value'])
+            elif data['type'] == 'ZFS':
+                if data['enabled']:
+                    await self.middleware.call('tunable.set_zfs_parameter', data['var'], data['value'])
+                    if update_initramfs:
+                        await self.middleware.call('boot.update_initramfs')
+            else:
+                await self.handle_tunable_change(data)
+        except Exception:
+            await self.middleware.call('datastore.delete', self._config.datastore, id_)
+            raise
 
         return await self.get_instance(id_)
 
@@ -206,23 +210,29 @@ async def do_update(self, job, id_, data):
             'datastore.update', self._config.datastore, id_, new, {'prefix': self._config.datastore_prefix}
         )
 
-        if new['type'] == 'SYSCTL':
-            await self.middleware.call('etc.generate', 'sysctl')
+        try:
+            if new['type'] == 'SYSCTL':
+                await self.middleware.call('etc.generate', 'sysctl')
 
-            if new['enabled']:
-                await self.middleware.call('tunable.set_sysctl', new['var'], new['value'])
-            else:
-                await self.middleware.call('tunable.reset_sysctl', new)
-        elif new['type'] == 'ZFS':
-            if new['enabled']:
-                await self.middleware.call('tunable.set_zfs_parameter', new['var'], new['value'])
-            else:
-                await self.middleware.call('tunable.reset_zfs_parameter', new)
+                if new['enabled']:
+                    await self.middleware.call('tunable.set_sysctl', new['var'], new['value'])
+                else:
+                    await self.middleware.call('tunable.reset_sysctl', new)
+            elif new['type'] == 'ZFS':
+                if new['enabled']:
+                    await self.middleware.call('tunable.set_zfs_parameter', new['var'], new['value'])
+                else:
+                    await self.middleware.call('tunable.reset_zfs_parameter', new)
 
-            if update_initramfs:
-                await self.middleware.call('boot.update_initramfs')
-        else:
-            await self.handle_tunable_change(new)
+                if update_initramfs:
+                    await self.middleware.call('boot.update_initramfs')
+            else:
+                await self.handle_tunable_change(new)
+        except Exception:
+            await self.middleware.call(
+                'datastore.update', self._config.datastore, id_, old, {'prefix': self._config.datastore_prefix}
+            )
+            raise
 
         return await self.get_instance(id_)
 
diff --git a/tests/api2/test_tunables.py b/tests/api2/test_tunables.py
index 2b57348139a0c..0b98f40ff02c0 100644
--- a/tests/api2/test_tunables.py
+++ b/tests/api2/test_tunables.py
@@ -162,3 +162,40 @@ def test_arc_max_set():
 
     mount_info = call("filesystem.mount_info", [["mountpoint", "=", "/"]], {"get": True})
     assert "RO" in mount_info["super_opts"]
+
+
+def test_create_error():
+    assert call("tunable.query") == []
+
+    with pytest.raises(Exception) as ve:
+        call("tunable.create", {
+            "type": "SYSCTL",
+            "var": "kernel.watchdog",
+            "value": "6",
+        }, job=True)
+
+    assert "Invalid argument" in str(ve.value)
+
+    assert call("tunable.query") == []
+
+
+def test_update_error():
+    assert call("tunable.query") == []
+
+    tunable = call("tunable.create", {
+        "type": "SYSCTL",
+        "var": "kernel.watchdog",
+        "value": "0",
+    }, job=True)
+
+    try:
+        with pytest.raises(Exception) as ve:
+            call("tunable.update", tunable["id"], {
+                "value": "6",
+            }, job=True)
+
+        assert "Invalid argument" in str(ve.value)
+
+        assert call("tunable.get_instance", tunable["id"])["value"] == "0"
+    finally:
+        call("tunable.delete", tunable["id"], job=True)