diff --git a/app/handlers/handlers_connectors.py b/app/handlers/handlers_connectors.py index 3b483ea7..d6aa687d 100644 --- a/app/handlers/handlers_connectors.py +++ b/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) diff --git a/app/handlers/handlers_resource.py b/app/handlers/handlers_resource.py index a3b4894d..ab709802 100644 --- a/app/handlers/handlers_resource.py +++ b/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") diff --git a/app/handlers/tests/test_handlers_connector.py b/app/handlers/tests/test_handlers_connector.py index cf409f4f..c539b10d 100644 --- a/app/handlers/tests/test_handlers_connector.py +++ b/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, } diff --git a/app/handlers/tests/test_handlers_resource.py b/app/handlers/tests/test_handlers_resource.py index d09c49b2..7370cc3b 100644 --- a/app/handlers/tests/test_handlers_resource.py +++ b/app/handlers/tests/test_handlers_resource.py @@ -55,21 +55,12 @@ 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, @@ -77,7 +68,7 @@ def test_update(self, mock_api_client): "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):