Merge branch 'main' into feature/oss-25-publish-versioned-chart

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 
 
+## v0.11.2 (2024-05-12)
+
+### Fix
+
+* fix: TwingateResourceAccess only updating every 10h and not immediately (#283) ([`2809bb9`](https://github.com/Twingate/kubernetes-operator/commit/2809bb99417370d767077ed082848038abd03273))
+
+
 ## v0.11.1 (2024-05-08)
 
 ### Fix

app/api/tests/test_client_connectors.py

@@ -258,3 +258,32 @@ def test_connector_delete_with_id_already_deleted_returns_true(
         )
         result = api_client.connector_delete("some-id")
         assert result is True
+
+    def test_connector_delete_raises_if_unknown_error(
+        self, test_url, api_client, mocked_responses
+    ):
+        failed_response = json.dumps(
+            {
+                "data": {
+                    "connectorDelete": {
+                        "ok": False,
+                        "error": "Unknown error...",
+                    }
+                }
+            }
+        )
+
+        mocked_responses.post(
+            test_url,
+            status=200,
+            body=failed_response,
+            match=[
+                responses.matchers.json_params_matcher(
+                    {"variables": {"id": "some-id"}}, strict_match=False
+                )
+            ],
+        )
+        with pytest.raises(
+            GraphQLMutationError, match="connectorDelete mutation failed."
+        ):
+            api_client.connector_delete("some-id")

app/api/tests/test_client_resources.py

@@ -312,3 +312,32 @@ def test_resource_delete_with_id_already_deleted_returns_true(
         )
         result = api_client.resource_delete("some-id")
         assert result is True
+
+    def test_resource_delete_raises_if_unknown_error(
+        self, test_url, api_client, mocked_responses
+    ):
+        failed_response = json.dumps(
+            {
+                "data": {
+                    "resourceDelete": {
+                        "ok": False,
+                        "error": "Something unknown happened...",
+                    }
+                }
+            }
+        )
+
+        mocked_responses.post(
+            test_url,
+            status=200,
+            body=failed_response,
+            match=[
+                responses.matchers.json_params_matcher(
+                    {"variables": {"id": "some-id"}}, strict_match=False
+                )
+            ],
+        )
+        with pytest.raises(
+            GraphQLMutationError, match="resourceDelete mutation failed."
+        ):
+            api_client.resource_delete("some-id")

app/handlers/handlers_resource_access.py

@@ -20,7 +20,7 @@ def get_principal_id(
         return principal_id
 
     if ref := access_crd.principal_external_ref:
-        # Once `twingate_resource_access_sync` ran and we have the principal_id
+        # Once `twingate_resource_access_change` ran and we have the principal_id
         # we dont use it and do not re-query the API
         if principal_id_already_fetched := create_status and create_status.get(
             "principal_id"
@@ -45,14 +45,16 @@ def get_principal_id(
 def check_status_created(status: dict | None) -> dict | None:
     if (
         create_status := status
-        and status.get(twingate_resource_access_sync.__name__, {})
+        and status.get(twingate_resource_access_change.__name__, {})
     ) and create_status["success"]:
         return create_status
 
     return None
 
 
-def twingate_resource_access_sync(body, spec, memo, logger, patch, status, **kwargs):
+@kopf.on.create("twingateresourceaccess")
+@kopf.on.update("twingateresourceaccess", field="spec")
+def twingate_resource_access_change(body, spec, memo, logger, patch, status, **kwargs):
     logger.info("Got a TwingateResourceAccess create request: %s", spec)
     creation_status = check_status_created(status)
 
@@ -79,9 +81,6 @@ def twingate_resource_access_sync(body, spec, memo, logger, patch, status, **kwa
             reason="Success",
             message=f"Added access to {resource_crd.spec.id}<>{principal_id}",
         )
-        patch.metadata["ownerReferences"] = [
-            resource_crd.metadata.owner_reference_object
-        ]
         return success(principal_id=principal_id, resource_id=resource_id)
     except GraphQLMutationError as mex:
         kopf.exception(
@@ -90,15 +89,16 @@ def twingate_resource_access_sync(body, spec, memo, logger, patch, status, **kwa
         return fail(error=mex.error)
 
 
-# can't use decorator syntax because typecheck would fail (update has some extra params that we're not using)
-kopf.on.create("twingateresourceaccess")(twingate_resource_access_sync)
-kopf.on.update("twingateresourceaccess", field="spec")(twingate_resource_access_sync)
-kopf.timer(
+@kopf.timer(
     "twingateresourceaccess",
     interval=timedelta(hours=10).seconds,
     initial_delay=60,
     idle=60,
-)(twingate_resource_access_sync)
+)
+def twingate_resource_access_sync(body, spec, memo, logger, patch, status, **kwargs):
+    return twingate_resource_access_change(
+        body, spec, memo, logger, patch, status, **kwargs
+    )
 
 
 @kopf.on.delete("twingateresourceaccess")

app/handlers/handlers_services.py

@@ -52,20 +52,19 @@ def service_to_twingate_resource(service_body, namespace) -> dict:
         if value := meta.annotations.get(f"twingate.com/resource-{key}"):
             result["spec"][key] = convert_f(value)
 
-    if service_ports := spec.get("ports", []):
-        protocols: dict = {
-            "allowIcmp": False,
-            "tcp": {"policy": "RESTRICTED", "ports": []},
-            "udp": {"policy": "RESTRICTED", "ports": []},
-        }
-        for port_obj in service_ports:
-            port = port_obj["port"]
-            if port_obj["protocol"] == "TCP":
-                protocols["tcp"]["ports"].append({"start": port, "end": port})
-            elif port_obj["protocol"] == "UDP":
-                protocols["udp"]["ports"].append({"start": port, "end": port})
-
-        result["spec"]["protocols"] = protocols
+    protocols: dict = {
+        "allowIcmp": False,
+        "tcp": {"policy": "RESTRICTED", "ports": []},
+        "udp": {"policy": "RESTRICTED", "ports": []},
+    }
+    for port_obj in spec.get("ports", []):
+        port = port_obj["port"]
+        if port_obj["protocol"] == "TCP":
+            protocols["tcp"]["ports"].append({"start": port, "end": port})
+        elif port_obj["protocol"] == "UDP":
+            protocols["udp"]["ports"].append({"start": port, "end": port})
+
+    result["spec"]["protocols"] = protocols
 
     return result
 

app/handlers/tests/test_handlers.py

@@ -0,0 +1,17 @@
+from datetime import timedelta
+
+import kopf
+
+
+def test_timer_handlers_minimum_5h():
+    # For safe-keeping - during debug someone might change timer
+    exceptions = ["twingate_connector_pod_reconciler"]
+
+    registry = kopf.get_default_registry()
+    spawning_handlers = registry._spawning  # noqa: SLF001
+    for handler in spawning_handlers._handlers:  # noqa: SLF001
+        if (
+            isinstance(handler, kopf._core.intents.handlers.TimerHandler)  # noqa: SLF001
+            and handler.id not in exceptions
+        ):
+            assert handler.interval > timedelta(hours=5).seconds

app/handlers/tests/test_handlers_resource_access.py

@@ -103,7 +103,7 @@ def test_from_external_ref_uses_created_status_principal_id(self):
         assert principal_id == expected
 
 
-class TestResourceAccessCreateHandler:
+class TestResourceAccessChangeHandler:
     def test_create_success(self, resource_factory, kopf_info_mock, mock_api_client):
         resource = resource_factory()
         resource_spec = resource.to_spec()
@@ -119,7 +119,6 @@ def test_create_success(self, resource_factory, kopf_info_mock, mock_api_client)
         memo_mock = MagicMock()
         patch_mock = MagicMock()
         patch_mock.metadata = {}
-        patch_mock.metadata["ownerReferences"] = []
 
         resource_crd_mock = MagicMock()
         resource_crd_mock.spec = resource_spec
@@ -145,14 +144,6 @@ def test_create_success(self, resource_factory, kopf_info_mock, mock_api_client)
             }
 
         kopf_info_mock.assert_called_once_with("", reason="Success", message=ANY)
-        assert patch_mock.metadata["ownerReferences"] == [
-            {
-                "apiVersion": "twingate.com/v1",
-                "kind": "TwingateResource",
-                "name": "foo",
-                "uid": "uid",
-            }
-        ]
 
     def test_create_invalid_ref(self, mock_api_client):
         resource_access_spec = {
@@ -285,7 +276,7 @@ def test_delete_success(self, resource_factory, mock_api_client):
         resource_crd_mock.metadata = K8sMetadata(uid="uid", name="foo", namespace="bar")
 
         status = {
-            "twingate_resource_access_sync": {
+            "twingate_resource_access_change": {
                 "success": True,
                 "principal_id": resource_access_spec["principalId"],
             }
@@ -312,7 +303,7 @@ def test_delete_resource_doesnt_exist_does_nothing(self, mock_api_client):
         logger_mock = MagicMock()
         memo_mock = MagicMock()
         status = {
-            "twingate_resource_access_sync": {
+            "twingate_resource_access_change": {
                 "success": True,
                 "principal_id": resource_access_spec["principalId"],
             }

app/handlers/tests/test_handlers_services.py

@@ -42,9 +42,7 @@ def example_service_body():
           targetPort: 9376
           name: ssh
     """
-    return kopf._cogs.structs.bodies.Body(  # noqa: SLF001
-        yaml.safe_load(yaml_str)
-    )
+    return kopf.Body(yaml.safe_load(yaml_str))
 
 
 @pytest.fixture()

app/version_policy_providers/tests/test_base.py

@@ -1,6 +1,26 @@
 import pytest
 
-from app.version_policy_providers import get_provider
+from app.version_policy_providers import (
+    DockerhubVersionPolicyProvider,
+    GoogleVersionPolicyProvider,
+    get_provider,
+)
+
+
+@pytest.mark.parametrize(
+    ("value", "repo", "expected"),
+    [
+        ("dockerhub", "twingate/test", DockerhubVersionPolicyProvider),
+        (
+            "google",
+            "us-docker.pkg.dev/proj/d/connector",
+            GoogleVersionPolicyProvider,
+        ),
+    ],
+)
+def test_get_provider_valid_value(value, repo, expected):
+    provider = get_provider(value, repo)
+    assert isinstance(provider, expected)
 
 
 def test_get_provider_with_invalid_value_raises():