fix: Resource restricted policy should allow empty ports (#193)

Twingate · Mar 11, 2024 · e9aec7d · e9aec7d
1 parent 7be86df
commit e9aec7d
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 79 deletions.
diff --git a/app/crds.py b/app/crds.py
@@ -63,13 +63,13 @@ class ProtocolPolicy(str, Enum):
     RESTRICTED = "RESTRICTED"
 
 
-class ProtocoRange(BaseModel):
+class ProtocolRange(BaseModel):
     model_config = ConfigDict(
         frozen=True, populate_by_name=True, alias_generator=to_camel
     )
 
-    start: int = Field(ge=0, le=65535)
-    end: int = Field(ge=0, le=65535)
+    start: int = Field(ge=1, le=65535)
+    end: int = Field(ge=1, le=65535)
 
     @model_validator(mode="after")
     def check_ports(self):
@@ -85,16 +85,13 @@ class ResourceProtocol(BaseModel):
     )
 
     policy: ProtocolPolicy = ProtocolPolicy.ALLOW_ALL
-    ports: list[ProtocoRange] = Field(default_factory=list)
+    ports: list[ProtocolRange] = Field(default_factory=list)
 
     @model_validator(mode="after")
     def check_policy_ports(self):
         if self.policy == ProtocolPolicy.ALLOW_ALL and self.ports:
             raise ValueError("ports can't be set if policy is ALLOW_ALL")
 
-        if self.policy == ProtocolPolicy.RESTRICTED and not self.ports:
-            raise ValueError("ports must be set if policy is RESTRICTED")
-
         return self
 
 

diff --git a/app/tests/test_crds_resource.py b/app/tests/test_crds_resource.py
@@ -126,18 +126,6 @@ def test_resourceprotocols_validation():
             },
         )
 
-    with pytest.raises(ValueError, match="ports must be set"):
-        TwingateResourceCRD(
-            apiVersion="twingate.com/v1",
-            kind="TwingateResource",
-            spec={
-                "address": "my.default.cluster.local",
-                "id": "UmVzb3VyY2U6OTM3Mzkw",
-                "name": "My K8S Resource",
-                "protocols": {"tcp": {"policy": "RESTRICTED"}},
-            },
-        )
-
 
 def test_resourceprotocol_ports_validation():
     with pytest.raises(ValueError, match="Input should be less than or equal to 65535"):
@@ -157,7 +145,7 @@ def test_resourceprotocol_ports_validation():
             },
         )
 
-    with pytest.raises(ValueError, match="Input should be greater than or equal to 0"):
+    with pytest.raises(ValueError, match="Input should be greater than or equal to 1"):
         TwingateResourceCRD(
             apiVersion="twingate.com/v1",
             kind="TwingateResource",

diff --git a/deploy/twingate-operator/crds/twingate.com.twingateresources.yaml b/deploy/twingate-operator/crds/twingate.com.twingateresources.yaml
@@ -62,8 +62,8 @@ spec:
                       nullable: true
                       description: "tcp specifies the resource's TCP protocol policy."
                       x-kubernetes-validations:
-                        - rule: (self.policy == "ALLOW_ALL" && size(self.ports) == 0) || (self.policy == "RESTRICTED" && size(self.ports) > 0)
-                          message: "Can't specify port ranges for ALLOW_ALL policy, and must specify port ranges for RESTRICTED policy"
+                        - rule: (self.policy == "ALLOW_ALL" && size(self.ports) == 0) || (self.policy == "RESTRICTED")
+                          message: "Can't specify port ranges for ALLOW_ALL policy."
                       properties:
                          policy:
                            type: string
@@ -83,20 +83,20 @@ spec:
                                start:
                                  type: integer
                                  nullable: false
-                                 minimum: 0
+                                 minimum: 1
                                  maximum: 65535
                                end:
                                  type: integer
                                  nullable: false
-                                 minimum: 0
+                                 minimum: 1
                                  maximum: 65535
                     udp:
                       type: object
                       nullable: true
                       description: "udp specifies the resource's UDP protocol policy."
                       x-kubernetes-validations:
-                        - rule: (self.policy == "ALLOW_ALL" && size(self.ports) == 0) || (self.policy == "RESTRICTED" && size(self.ports) > 0)
-                          message: "Can't specify port ranges for ALLOW_ALL policy, and must specify port ranges for RESTRICTED policy"
+                        - rule: (self.policy == "ALLOW_ALL" && size(self.ports) == 0) || (self.policy == "RESTRICTED")
+                          message: "Can't specify port ranges for ALLOW_ALL policy."
                       properties:
                          policy:
                            type: string
@@ -116,12 +116,12 @@ spec:
                                start:
                                  type: integer
                                  nullable: false
-                                 minimum: 0
+                                 minimum: 1
                                  maximum: 65535
                                end:
                                  type: integer
                                  nullable: false
-                                 minimum: 0
+                                 minimum: 1
                                  maximum: 65535
             status:
               type: object

diff --git a/tests_integration/test_connector_flows.py b/tests_integration/test_connector_flows.py
@@ -79,7 +79,7 @@ def test_connector_flows(kopf_settings, kopf_runner_args, ci_run_number):
         assert pod["metadata"]["ownerReferences"][0]["kind"] == "TwingateConnector"
 
         kubectl_delete(f"tc/{connector_name}")
-        time.sleep(5)
+        time.sleep(10)
 
         # secret & pod are deleted
         with pytest.raises(CalledProcessError):

diff --git a/tests_integration/test_crds_resource.py b/tests_integration/test_crds_resource.py
@@ -86,10 +86,7 @@ def test_protocols_tcp_allowall_cant_specify_ports(unique_resource_name):
         )
 
     stderr = ex.value.stderr.decode()
-    assert (
-        "Can't specify port ranges for ALLOW_ALL policy, and must specify port ranges for RESTRICTED policy"
-        in stderr
-    )
+    assert "Can't specify port ranges for ALLOW_ALL policy." in stderr
 
     result = kubectl_create(
         f"""
@@ -131,10 +128,7 @@ def test_protocols_udp_allowall_cant_specify_ports(unique_resource_name):
         )
 
     stderr = ex.value.stderr.decode()
-    assert (
-        "Can't specify port ranges for ALLOW_ALL policy, and must specify port ranges for RESTRICTED policy"
-        in stderr
-    )
+    assert "Can't specify port ranges for ALLOW_ALL policy." in stderr
 
     result = kubectl_create(
         f"""
@@ -155,29 +149,25 @@ def test_protocols_udp_allowall_cant_specify_ports(unique_resource_name):
     kubectl_delete(f"tgr/{unique_resource_name}")
 
 
-def test_protocols_tcp_restricted_must_specify_ports(unique_resource_name):
-    with pytest.raises(subprocess.CalledProcessError) as ex:
-        kubectl_create(
-            f"""
-            apiVersion: twingate.com/v1beta
-            kind: TwingateResource
-            metadata:
-                name: {unique_resource_name}
-            spec:
-                name: My K8S Resource
-                address: "foo.default.cluster.local"
-                protocols:
-                    tcp:
-                        policy: RESTRICTED
-        """
-        )
-
-    stderr = ex.value.stderr.decode()
-    assert (
-        "Can't specify port ranges for ALLOW_ALL policy, and must specify port ranges for RESTRICTED policy"
-        in stderr
+def test_protocols_tcp_restricted(unique_resource_name):
+    result = kubectl_create(
+        f"""
+        apiVersion: twingate.com/v1beta
+        kind: TwingateResource
+        metadata:
+            name: {unique_resource_name}
+        spec:
+            name: My K8S Resource
+            address: "foo.default.cluster.local"
+            protocols:
+                tcp:
+                    policy: RESTRICTED
+    """
     )
 
+    assert result.returncode == 0, result.value.stderr.decode()
+    kubectl_delete(f"tgr/{unique_resource_name}")
+
     result = kubectl_create(
         f"""
         apiVersion: twingate.com/v1beta
@@ -200,29 +190,25 @@ def test_protocols_tcp_restricted_must_specify_ports(unique_resource_name):
     kubectl_delete(f"tgr/{unique_resource_name}")
 
 
-def test_protocols_udp_restricted_must_specify_ports(unique_resource_name):
-    with pytest.raises(subprocess.CalledProcessError) as ex:
-        kubectl_create(
-            f"""
-            apiVersion: twingate.com/v1beta
-            kind: TwingateResource
-            metadata:
-                name: {unique_resource_name}
-            spec:
-                name: My K8S Resource
-                address: "foo.default.cluster.local"
-                protocols:
-                    udp:
-                        policy: RESTRICTED
-        """
-        )
-
-    stderr = ex.value.stderr.decode()
-    assert (
-        "Can't specify port ranges for ALLOW_ALL policy, and must specify port ranges for RESTRICTED policy"
-        in stderr
+def test_protocols_udp_restricted(unique_resource_name):
+    result = kubectl_create(
+        f"""
+        apiVersion: twingate.com/v1beta
+        kind: TwingateResource
+        metadata:
+            name: {unique_resource_name}
+        spec:
+            name: My K8S Resource
+            address: "foo.default.cluster.local"
+            protocols:
+                udp:
+                    policy: RESTRICTED
+    """
     )
 
+    assert result.returncode == 0, result.value.stderr.decode()
+    kubectl_delete(f"tgr/{unique_resource_name}")
+
     result = kubectl_create(
         f"""
         apiVersion: twingate.com/v1beta