Merge pull request #638 from averevki/defaults_overrides_merge_strategy

Add tests for D&O merge strategy

Author: averevki

testsuite/kuadrant/policy/__init__.py

@@ -1,11 +1,19 @@
 """Contains Base class for policies"""
 
 from dataclasses import dataclass
+from enum import Enum
 
 from testsuite.kubernetes import KubernetesObject
 from testsuite.utils import check_condition
 
 
+class Strategy(Enum):
+    """Class for merge strategies of defaults and overrides."""
+
+    ATOMIC = "atomic"
+    MERGE = "merge"
+
+
 @dataclass
 class CelPredicate:
     """Dataclass that references CEL predicate e.g. auth.identity.anonymous == 'true'"""

testsuite/kuadrant/policy/authorization/auth_policy.py

@@ -9,7 +9,7 @@
 from testsuite.utils import asdict
 from .auth_config import AuthConfig
 from .sections import ResponseSection
-from .. import Policy, CelPredicate
+from .. import Policy, CelPredicate, Strategy
 from . import Pattern
 
 
@@ -49,6 +49,15 @@ def add_rule(self, when: list[CelPredicate]):
         self.model.spec.setdefault("when", [])
         self.model.spec["when"].extend([asdict(x) for x in when])
 
+    @modify
+    def strategy(self, strategy: Strategy) -> None:
+        """Add strategy type to default or overrides spec"""
+        if self.spec_section is None:
+            raise TypeError("Strategy can only be set on defaults or overrides")
+
+        self.spec_section["strategy"] = strategy.value
+        self.spec_section = None
+
     @property
     def auth_section(self):
         if self.spec_section is None:

testsuite/kuadrant/policy/rate_limit.py

@@ -7,7 +7,7 @@
 from testsuite.gateway import Referencable
 from testsuite.kubernetes import modify
 from testsuite.kubernetes.client import KubernetesClient
-from testsuite.kuadrant.policy import Policy, CelPredicate, CelExpression
+from testsuite.kuadrant.policy import Policy, CelPredicate, CelExpression, Strategy
 from testsuite.utils import asdict
 
 
@@ -72,6 +72,15 @@ def add_limit(
         self.spec_section.setdefault("limits", {})[name] = limit
         self.spec_section = None
 
+    @modify
+    def strategy(self, strategy: Strategy) -> None:
+        """Add strategy type to default or overrides spec"""
+        if self.spec_section is None:
+            raise TypeError("Strategy can only be set on defaults or overrides")
+
+        self.spec_section["strategy"] = strategy.value
+        self.spec_section = None
+
     @property
     def defaults(self):
         """Add new rule into the `defaults` RateLimitPolicy section"""

testsuite/tests/singlecluster/defaults/merge/__init__.py

@@ -0,0 +1,12 @@
+"""Conftest for defaults merge strategy tests"""
+
+import pytest
+
+
+@pytest.fixture(scope="module")
+def route(route, backend):
+    """Add 2 backend rules for specific backend paths"""
+    route.remove_all_rules()
+    route.add_backend(backend, "/get")
+    route.add_backend(backend, "/anything")
+    return route

testsuite/tests/singlecluster/defaults/merge/conftest.py

@@ -0,0 +1,28 @@
+"""Setup conftest for policy merge on the same targets"""
+
+import pytest
+
+from testsuite.kuadrant.policy import CelPredicate, Strategy
+from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy
+
+LIMIT = Limit(4, "10s")
+MERGE_LIMIT = Limit(2, "10s")
+MERGE_LIMIT2 = Limit(6, "10s")
+
+
+@pytest.fixture(scope="module")
+def rate_limit(cluster, blame, module_label, route):
+    """Add a RateLimitPolicy targeting the first HTTPRouteRule."""
+    rate_limit = RateLimitPolicy.create_instance(cluster, blame("sp"), route, labels={"testRun": module_label})
+    rate_limit.add_limit("basic", [LIMIT], when=[CelPredicate("request.path == '/get'")])
+    return rate_limit
+
+
+@pytest.fixture(scope="module")
+def default_merge_rate_limit(cluster, blame, module_label, route):
+    """Add a RateLimitPolicy targeting the first HTTPRouteRule."""
+    policy = RateLimitPolicy.create_instance(cluster, blame("dmp"), route, labels={"testRun": module_label})
+    policy.defaults.add_limit("basic", [MERGE_LIMIT], when=[CelPredicate("request.path == '/get'")])
+    policy.defaults.add_limit("merge", [MERGE_LIMIT2], when=[CelPredicate("request.path == '/anything'")])
+    policy.defaults.strategy(Strategy.MERGE)
+    return policy

testsuite/tests/singlecluster/defaults/merge/same_target/__init__.py

@@ -0,0 +1,38 @@
+"""Test defaults policy aimed at the same resource uses the oldest policy."""
+
+import pytest
+
+from testsuite.kuadrant.policy import has_condition
+from .conftest import MERGE_LIMIT, MERGE_LIMIT2
+
+pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador]
+
+
+@pytest.fixture(scope="module", autouse=True)
+def commit(request, route, rate_limit, default_merge_rate_limit):  # pylint: disable=unused-argument
+    """Commits RateLimitPolicy after the HTTPRoute is created"""
+    for policy in [rate_limit, default_merge_rate_limit]:  # Forcing order of creation.
+        request.addfinalizer(policy.delete)
+        policy.commit()
+        policy.wait_for_accepted()
+
+
+def test_multiple_policies_merge_default_ab(client, rate_limit, default_merge_rate_limit):
+    """Test RateLimitPolicy with merge defaults being enforced due to age"""
+    assert rate_limit.wait_until(
+        has_condition(
+            "Enforced",
+            "False",
+            "Overridden",
+            "RateLimitPolicy is overridden by "
+            f"[{default_merge_rate_limit.namespace()}/{default_merge_rate_limit.name()}]",
+        )
+    )
+
+    responses = client.get_many("/get", MERGE_LIMIT.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/get").status_code == 429
+
+    responses = client.get_many("/anything", MERGE_LIMIT2.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/anything").status_code == 429

testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py

@@ -0,0 +1,32 @@
+"""Test defaults policy aimed at the same resource uses the oldest policy."""
+
+import pytest
+
+from testsuite.kuadrant.policy import has_condition
+from .conftest import LIMIT, MERGE_LIMIT2
+
+pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador]
+
+
+@pytest.fixture(scope="module", autouse=True)
+def commit(request, route, rate_limit, default_merge_rate_limit):  # pylint: disable=unused-argument
+    """Commits RateLimitPolicy after the HTTPRoute is created"""
+    for policy in [default_merge_rate_limit, rate_limit]:  # Forcing order of creation.
+        request.addfinalizer(policy.delete)
+        policy.commit()
+        policy.wait_for_accepted()
+
+
+def test_multiple_policies_merge_default_ba(client, default_merge_rate_limit):
+    """Test RateLimitPolicy with merge defaults being ignored due to age"""
+    assert default_merge_rate_limit.wait_until(
+        has_condition("Enforced", "True", "Enforced", "RateLimitPolicy has been partially enforced")
+    )
+
+    responses = client.get_many("/get", LIMIT.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/get").status_code == 429
+
+    responses = client.get_many("/anything", MERGE_LIMIT2.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/anything").status_code == 429

testsuite/tests/singlecluster/defaults/merge/same_target/test_ab_strategy.py

@@ -0,0 +1,55 @@
+"""Test gateway level default merging with and being partially overriden by another policy."""
+
+import pytest
+
+from testsuite.kuadrant.policy import CelPredicate, has_condition
+from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit, Strategy
+
+pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador]
+
+
+@pytest.fixture(scope="module")
+def rate_limit(rate_limit):
+    """Create a RateLimitPolicy with a basic limit with same target as one default."""
+    when = CelPredicate("request.path == '/get'")
+    rate_limit.add_limit("route_limit", [Limit(5, "5s")], when=[when])
+    return rate_limit
+
+
+@pytest.fixture(scope="module")
+def global_rate_limit(cluster, blame, module_label, gateway):
+    """Create a RateLimitPolicy with default policies and a merge strategy."""
+    global_rate_limit = RateLimitPolicy.create_instance(
+        cluster, blame("limit"), gateway, labels={"testRun": module_label}
+    )
+    gateway_when = CelPredicate("request.path == '/anything'")
+    global_rate_limit.defaults.add_limit("gateway_limit", [Limit(3, "5s")], when=[gateway_when])
+    route_when = CelPredicate("request.path == '/get'")
+    global_rate_limit.defaults.add_limit("route_limit", [Limit(10, "5s")], when=[route_when])
+    global_rate_limit.defaults.strategy(Strategy.MERGE)
+    return global_rate_limit
+
+
+@pytest.fixture(scope="module", autouse=True)
+def commit(request, route, rate_limit, global_rate_limit):  # pylint: disable=unused-argument
+    """Commits RateLimitPolicy after the HTTPRoute is created"""
+    for policy in [global_rate_limit, rate_limit]:  # Forcing order of creation.
+        request.addfinalizer(policy.delete)
+        policy.commit()
+        policy.wait_for_ready()
+
+
+@pytest.mark.parametrize("rate_limit", ["gateway", "route"], indirect=True)
+def test_gateway_default_merge(client, global_rate_limit):
+    """Test Gateway default policy being partially overriden when another policy with the same target is created."""
+    assert global_rate_limit.wait_until(
+        has_condition("Enforced", "True", "Enforced", "RateLimitPolicy has been partially enforced")
+    )
+
+    get = client.get_many("/get", 5)
+    get.assert_all(status_code=200)
+    assert client.get("/get").status_code == 429
+
+    anything = client.get_many("/anything", 3)
+    anything.assert_all(status_code=200)
+    assert client.get("/anything").status_code == 429

testsuite/tests/singlecluster/defaults/merge/same_target/test_ba_startegy.py

@@ -0,0 +1,12 @@
+"""Conftest for overrides merge strategy tests"""
+
+import pytest
+
+
+@pytest.fixture(scope="module")
+def route(route, backend):
+    """Add 2 backend rules for specific backend paths"""
+    route.remove_all_rules()
+    route.add_backend(backend, "/get")
+    route.add_backend(backend, "/anything")
+    return route

testsuite/tests/singlecluster/defaults/merge/test_gateway_default_merge.py

@@ -0,0 +1,28 @@
+"""Setup conftest for policy override on the same targets"""
+
+import pytest
+
+from testsuite.kuadrant.policy import CelPredicate, Strategy
+from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy
+
+LIMIT = Limit(4, "5s")
+OVERRIDE_LIMIT = Limit(6, "5s")
+OVERRIDE_LIMIT2 = Limit(2, "5s")
+
+
+@pytest.fixture(scope="module")
+def rate_limit(cluster, blame, module_label, route):
+    """Add a RateLimitPolicy targeting the first HTTPRouteRule."""
+    rate_limit = RateLimitPolicy.create_instance(cluster, blame("sp"), route, labels={"testRun": module_label})
+    rate_limit.add_limit("basic", [LIMIT], when=[CelPredicate("request.path == '/get'")])
+    return rate_limit
+
+
+@pytest.fixture(scope="module")
+def override_merge_rate_limit(cluster, blame, module_label, route):
+    """Add a RateLimitPolicy targeting the first HTTPRouteRule."""
+    policy = RateLimitPolicy.create_instance(cluster, blame("omp"), route, labels={"testRun": module_label})
+    policy.overrides.add_limit("basic", [OVERRIDE_LIMIT], when=[CelPredicate("request.path == '/get'")])
+    policy.overrides.add_limit("override", [OVERRIDE_LIMIT2], when=[CelPredicate("request.path == '/anything'")])
+    policy.overrides.strategy(Strategy.MERGE)
+    return policy

testsuite/tests/singlecluster/overrides/merge/__init__.py

@@ -0,0 +1,38 @@
+"""Test override policy aimed at the same resource always takes precedence."""
+
+import pytest
+
+from testsuite.kuadrant.policy import has_condition
+from .conftest import OVERRIDE_LIMIT, OVERRIDE_LIMIT2
+
+pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador]
+
+
+@pytest.fixture(scope="module", autouse=True)
+def commit(request, route, rate_limit, override_merge_rate_limit):  # pylint: disable=unused-argument
+    """Commits RateLimitPolicy after the HTTPRoute is created"""
+    for policy in [rate_limit, override_merge_rate_limit]:  # Forcing order of creation.
+        request.addfinalizer(policy.delete)
+        policy.commit()
+        policy.wait_for_accepted()
+
+
+def test_multiple_policies_merge_default_ab(client, rate_limit, override_merge_rate_limit):
+    """Test RateLimitPolicy with merge overrides always being enforced"""
+    assert rate_limit.wait_until(
+        has_condition(
+            "Enforced",
+            "False",
+            "Overridden",
+            "RateLimitPolicy is overridden by "
+            f"[{override_merge_rate_limit.namespace()}/{override_merge_rate_limit.name()}]",
+        )
+    )
+
+    responses = client.get_many("/get", OVERRIDE_LIMIT.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/get").status_code == 429
+
+    responses = client.get_many("/anything", OVERRIDE_LIMIT2.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/anything").status_code == 429

testsuite/tests/singlecluster/overrides/merge/conftest.py

@@ -0,0 +1,38 @@
+"""Test override policy aimed at the same resource always takes precedence."""
+
+import pytest
+
+from testsuite.kuadrant.policy import has_condition
+from .conftest import OVERRIDE_LIMIT, OVERRIDE_LIMIT2
+
+pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador]
+
+
+@pytest.fixture(scope="module", autouse=True)
+def commit(request, route, rate_limit, override_merge_rate_limit):  # pylint: disable=unused-argument
+    """Commits RateLimitPolicy after the HTTPRoute is created"""
+    for policy in [override_merge_rate_limit, rate_limit]:  # Forcing order of creation.
+        request.addfinalizer(policy.delete)
+        policy.commit()
+        policy.wait_for_accepted()
+
+
+def test_multiple_policies_merge_default_ba(client, rate_limit, override_merge_rate_limit):
+    """Test RateLimitPolicy with merge overrides always being enforced"""
+    assert rate_limit.wait_until(
+        has_condition(
+            "Enforced",
+            "False",
+            "Overridden",
+            "RateLimitPolicy is overridden by "
+            f"[{override_merge_rate_limit.namespace()}/{override_merge_rate_limit.name()}]",
+        )
+    )
+
+    responses = client.get_many("/get", OVERRIDE_LIMIT.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/get").status_code == 429
+
+    responses = client.get_many("/anything", OVERRIDE_LIMIT2.limit)
+    responses.assert_all(status_code=200)
+    assert client.get("/anything").status_code == 429