fix: Reduce update scope for twingate_resource_update

app/handlers/handlers_connectors.py

@@ -168,7 +168,7 @@ def twingate_connector_update(body, memo, logger, new, diff, status, **_):
 
     crd = TwingateConnectorCRD(**body)
     if not crd.spec.id:
-        return fail(reason="Update called before Connector has an ID")
+        return fail(error="Update called before Connector has an ID")
 
     updated_connector = client.connector_update(crd.spec)
     return success(twingate_id=updated_connector.id)

app/handlers/handlers_resource.py

@@ -4,7 +4,7 @@
 
 from app.api import TwingateAPIClient
 from app.crds import ResourceSpec
-from app.handlers.base import success
+from app.handlers.base import fail, success
 
 
 @kopf.on.create("twingateresource")
@@ -21,26 +21,32 @@ def twingate_resource_create(body, spec, memo, logger, patch, **kwargs):
     )
 
 
-@kopf.on.update("twingateresource")
-def twingate_resource_update(old, new, diff, status, memo, logger, **kwargs):
+@kopf.on.update("twingateresource", field="spec")
+def twingate_resource_update(spec, diff, status, memo, logger, **kwargs):
     logger.info(
         "Got TwingateResource update request: %s. Diff: %s. Status: %s.",
-        new,
+        spec,
         diff,
         status,
     )
 
-    crd = ResourceSpec(**new["spec"])
-    if crd.id:
-        logger.info("Updating resource %s", crd.id)
-        client = TwingateAPIClient(memo.twingate_settings)
-        resource = client.resource_update(crd)
-        logger.info("Got resource %s", resource)
-        return success(
-            twingate_id=resource.id,
-            created_at=resource.created_at.isoformat(),
-            updated_at=resource.updated_at.isoformat(),
-        )
+    crd = ResourceSpec(**spec)
+    if not crd.id:
+        return fail(error="Resource ID is missing in the spec")
+
+    # Check if just "id" was added - means `create` just ran
+    if len(diff) == 1 and diff[0][:3] == ("add", ("id",), None):
+        return success(twingate_id=crd.id, message="No update required")
+
+    logger.info("Updating resource %s", crd.id)
+    client = TwingateAPIClient(memo.twingate_settings)
+    resource = client.resource_update(crd)
+    logger.info("Got resource %s", resource)
+    return success(
+        twingate_id=resource.id,
+        created_at=resource.created_at.isoformat(),
+        updated_at=resource.updated_at.isoformat(),
+    )
 
 
 @kopf.on.delete("twingateresource")

app/handlers/tests/test_handlers_connector.py

@@ -222,7 +222,7 @@ def test_twingate_connector_update_without_id_does_nothing(
         twingate_connector_update, crd, MagicMock(), new={}, diff={}
     )
     assert run.result == {
-        "reason": "Update called before Connector has an ID",
+        "error": "Update called before Connector has an ID",
         "success": False,
         "ts": ANY,
     }

app/handlers/tests/test_handlers_resource.py

@@ -55,29 +55,20 @@ def test_create(self, resource_factory, kopf_info_mock, mock_api_client):
 class TestResourceUpdateHandler:
     def test_update(self, mock_api_client):
         rid = "UmVzb3VyY2U6OTMxODE3"
-        old = {
-            "spec": {
-                "id": rid,
-                "address": "my.default.cluster.local",
-                "name": "My K8S Resource",
-            }
-        }
-        new = {
-            "spec": {
-                "id": rid,
-                "address": "my.default.cluster.local",
-                "name": "new-name",
-            }
+        spec = new = {
+            "id": rid,
+            "address": "my.default.cluster.local",
+            "name": "new-name",
         }
-        diff = (("change", ("spec", "name"), "My K8S Resource", "new-name"),)
+        diff = (("change", ("name"), "My K8S Resource", "new-name"),)
         status = {
             "twingate_resource_create": {
                 "twingate_id": rid,
                 "created_at": "2023-09-27T04:02:55.249011+00:00",
                 "updated_at": "2023-09-27T04:02:55.249035+00:00",
             }
         }
-        new_resource_spec = ResourceSpec(**new["spec"])
+        new_resource_spec = ResourceSpec(**new)
 
         mock_api_client.resource_update.return_value = MagicMock(id=rid)
 
@@ -86,9 +77,7 @@ def test_update(self, mock_api_client):
         patch_mock = MagicMock()
         patch_mock.spec = {}
 
-        result = twingate_resource_update(
-            old, new, diff, status, memo_mock, logger_mock
-        )
+        result = twingate_resource_update(spec, diff, status, memo_mock, logger_mock)
         assert result == {
             "success": True,
             "twingate_id": rid,
@@ -100,6 +89,63 @@ def test_update(self, mock_api_client):
         mock_api_client.resource_update.assert_called_once_with(new_resource_spec)
         assert patch_mock.spec == {}
 
+    def test_update_called_without_id_fails(self, mock_api_client):
+        spec = {
+            "address": "my.default.cluster.local",
+            "name": "new-name",
+        }
+        diff = []
+        status = {}
+
+        logger_mock = MagicMock()
+        memo_mock = MagicMock()
+        patch_mock = MagicMock()
+        patch_mock.spec = {}
+
+        result = twingate_resource_update(spec, diff, status, memo_mock, logger_mock)
+        assert result == {
+            "success": False,
+            "error": "Resource ID is missing in the spec",
+            "ts": ANY,
+        }
+
+        mock_api_client.resource_update.assert_not_called()
+        assert patch_mock.spec == {}
+
+    def test_update_caused_by_create_does_nothing(self, mock_api_client):
+        rid = "UmVzb3VyY2U6OTMxODE3"
+        spec = {
+            "id": rid,
+            "address": "my.default.cluster.local",
+            "name": "new-name",
+        }
+        diff = (("add", ("id",), None, rid),)
+        status = {
+            "twingate_resource_create": {
+                "twingate_id": rid,
+                "created_at": "2023-09-27T04:02:55.249011+00:00",
+                "updated_at": "2023-09-27T04:02:55.249035+00:00",
+            }
+        }
+
+        mock_api_client.resource_update.return_value = MagicMock(id=rid)
+
+        logger_mock = MagicMock()
+        memo_mock = MagicMock()
+        patch_mock = MagicMock()
+        patch_mock.spec = {}
+
+        result = twingate_resource_update(spec, diff, status, memo_mock, logger_mock)
+        assert result == {
+            "success": True,
+            "twingate_id": rid,
+            "message": "No update required",
+            "ts": ANY,
+        }
+
+        mock_api_client.resource_update.assert_not_called()
+        assert patch_mock.spec == {}
+
 
 class TestResourceDeleteHandler:
     def test_delete(self, mock_api_client):