From 489cc184ba2a73db78378de6310eb7d8c3da86a9 Mon Sep 17 00:00:00 2001 From: Martin Hesko Date: Wed, 20 Nov 2024 14:55:20 +0100 Subject: [PATCH 1/8] added section_name parameter to all policy creation functions. Signed-off-by: Martin Hesko --- .../kuadrant/policy/authorization/auth_policy.py | 3 +++ testsuite/kuadrant/policy/dns.py | 3 +++ testsuite/kuadrant/policy/rate_limit.py | 13 +++++++++++-- testsuite/kuadrant/policy/tls.py | 5 ++++- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/testsuite/kuadrant/policy/authorization/auth_policy.py b/testsuite/kuadrant/policy/authorization/auth_policy.py index 4c77bfe4..62d2292f 100644 --- a/testsuite/kuadrant/policy/authorization/auth_policy.py +++ b/testsuite/kuadrant/policy/authorization/auth_policy.py @@ -27,6 +27,7 @@ def create_instance( name, target: Referencable, labels: Dict[str, str] = None, + section_name: str = None, ): """Creates base instance""" model: Dict = { @@ -37,6 +38,8 @@ def create_instance( "targetRef": target.reference, }, } + if section_name: + model["spec"]["targetRef"]["sectionName"] = section_name return cls(model, context=cluster.context) diff --git a/testsuite/kuadrant/policy/dns.py b/testsuite/kuadrant/policy/dns.py index d9c8d360..a2ec52fe 100644 --- a/testsuite/kuadrant/policy/dns.py +++ b/testsuite/kuadrant/policy/dns.py @@ -69,6 +69,7 @@ def create_instance( name: str, parent: Referencable, provider_secret_name: str, + section_name: str = None, load_balancing: LoadBalancing = None, labels: dict[str, str] = None, ): @@ -86,6 +87,8 @@ def create_instance( if load_balancing: model["spec"]["loadBalancing"] = asdict(load_balancing) + if section_name: + model["spec"]["targetRef"]["sectionName"] = section_name return cls(model, context=cluster.context) diff --git a/testsuite/kuadrant/policy/rate_limit.py b/testsuite/kuadrant/policy/rate_limit.py index a46ee1ea..7332f751 100644 --- a/testsuite/kuadrant/policy/rate_limit.py +++ b/testsuite/kuadrant/policy/rate_limit.py @@ -27,9 +27,16 @@ def __init__(self, *args, **kwargs): self.spec_section = None @classmethod - def create_instance(cls, cluster: KubernetesClient, name, target: Referencable, labels: dict[str, str] = None): + def create_instance( + cls, + cluster: KubernetesClient, + name, + target: Referencable, + section_name: str = None, + labels: dict[str, str] = None, + ): """Creates new instance of RateLimitPolicy""" - model = { + model: dict = { "apiVersion": "kuadrant.io/v1", "kind": "RateLimitPolicy", "metadata": {"name": name, "labels": labels}, @@ -37,6 +44,8 @@ def create_instance(cls, cluster: KubernetesClient, name, target: Referencable, "targetRef": target.reference, }, } + if section_name: + model["spec"]["targetRef"]["sectionName"] = section_name return cls(model, context=cluster.context) diff --git a/testsuite/kuadrant/policy/tls.py b/testsuite/kuadrant/policy/tls.py index 0fe0d296..0e2043eb 100644 --- a/testsuite/kuadrant/policy/tls.py +++ b/testsuite/kuadrant/policy/tls.py @@ -15,6 +15,7 @@ def create_instance( name: str, parent: Referencable, issuer: Referencable, + section_name: str = None, labels: dict[str, str] = None, commonName: str = None, duration: str = None, @@ -24,7 +25,7 @@ def create_instance( ): # pylint: disable=invalid-name """Creates new instance of TLSPolicy""" - model = { + model: dict = { "apiVersion": "kuadrant.io/v1", "kind": "TLSPolicy", "metadata": {"name": name, "labels": labels}, @@ -40,6 +41,8 @@ def create_instance( }, }, } + if section_name: + model["spec"]["targetRef"]["sectionName"] = section_name return cls(model, context=cluster.context) From 1eb61c19b8b8a3d0fa4fe75a99843a5644646ecb Mon Sep 17 00:00:00 2001 From: Martin Hesko Date: Wed, 20 Nov 2024 14:56:46 +0100 Subject: [PATCH 2/8] Updated RouteSelector tests to use SectionNames Signed-off-by: Martin Hesko --- .../route/test_limit_targeting_two_rules.py | 33 ------------ .../limitador/route/test_route_rule.py | 27 ---------- .../limitador/{route => section}/__init__.py | 0 .../limitador/{route => section}/conftest.py | 10 +++- .../limitador/section/test_listener.py | 24 +++++++++ .../limitador/section/test_multiple_rules.py | 51 +++++++++++++++++++ .../section/test_multiple_same_listener.py | 28 ++++++++++ .../test_multiple_same_rule.py | 18 +++---- .../limitador/section/test_route_rule.py | 26 ++++++++++ 9 files changed, 147 insertions(+), 70 deletions(-) delete mode 100644 testsuite/tests/singlecluster/limitador/route/test_limit_targeting_two_rules.py delete mode 100644 testsuite/tests/singlecluster/limitador/route/test_route_rule.py rename testsuite/tests/singlecluster/limitador/{route => section}/__init__.py (100%) rename testsuite/tests/singlecluster/limitador/{route => section}/conftest.py (58%) create mode 100644 testsuite/tests/singlecluster/limitador/section/test_listener.py create mode 100644 testsuite/tests/singlecluster/limitador/section/test_multiple_rules.py create mode 100644 testsuite/tests/singlecluster/limitador/section/test_multiple_same_listener.py rename testsuite/tests/singlecluster/limitador/{route => section}/test_multiple_same_rule.py (50%) create mode 100644 testsuite/tests/singlecluster/limitador/section/test_route_rule.py diff --git a/testsuite/tests/singlecluster/limitador/route/test_limit_targeting_two_rules.py b/testsuite/tests/singlecluster/limitador/route/test_limit_targeting_two_rules.py deleted file mode 100644 index 35548ff7..00000000 --- a/testsuite/tests/singlecluster/limitador/route/test_limit_targeting_two_rules.py +++ /dev/null @@ -1,33 +0,0 @@ -"""Tests that one RLP limit targeting two rules limits them together""" - -import pytest - -from testsuite.kuadrant.policy import CelPredicate -from testsuite.kuadrant.policy.rate_limit import Limit - - -pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] - - -@pytest.fixture(scope="module") -def rate_limit(rate_limit): - """Add limit to the policy""" - rate_limit.add_limit("test", [Limit(5, "10s")], when=[CelPredicate("request.method == 'GET'")]) - return rate_limit - - -@pytest.mark.issue("https://github.com/Kuadrant/testsuite/issues/561") -def test_limit_targeting_two_rules(client): - """Tests that one RLP limit targeting two rules limits them together""" - responses = client.get_many("/get", 3) - assert all( - r.status_code == 200 for r in responses - ), f"Rate Limited resource unexpectedly rejected requests {responses}" - - responses = client.get_many("/anything", 2) - assert all( - r.status_code == 200 for r in responses - ), f"Rate Limited resource unexpectedly rejected requests {responses}" - - assert client.get("/get").status_code == 429 - assert client.get("/anything").status_code == 429 diff --git a/testsuite/tests/singlecluster/limitador/route/test_route_rule.py b/testsuite/tests/singlecluster/limitador/route/test_route_rule.py deleted file mode 100644 index f199eb10..00000000 --- a/testsuite/tests/singlecluster/limitador/route/test_route_rule.py +++ /dev/null @@ -1,27 +0,0 @@ -"""Tests that the RLP is correctly apply to the route rule""" - -import pytest - -from testsuite.kuadrant.policy import CelPredicate -from testsuite.kuadrant.policy.rate_limit import Limit - -pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] - - -@pytest.fixture(scope="module") -def rate_limit(rate_limit): - """Add limit to the policy""" - rate_limit.add_limit("multiple", [Limit(5, "10s")], when=[CelPredicate("request.path == '/get'")]) - return rate_limit - - -@pytest.mark.issue("https://github.com/Kuadrant/testsuite/issues/561") -def test_rule_match(client): - """Tests that RLP correctly applies to the given HTTPRoute rule""" - responses = client.get_many("/get", 5) - responses.assert_all(status_code=200) - - assert client.get("/get").status_code == 429 - - response = client.get("/anything") - assert response.status_code == 200 diff --git a/testsuite/tests/singlecluster/limitador/route/__init__.py b/testsuite/tests/singlecluster/limitador/section/__init__.py similarity index 100% rename from testsuite/tests/singlecluster/limitador/route/__init__.py rename to testsuite/tests/singlecluster/limitador/section/__init__.py diff --git a/testsuite/tests/singlecluster/limitador/route/conftest.py b/testsuite/tests/singlecluster/limitador/section/conftest.py similarity index 58% rename from testsuite/tests/singlecluster/limitador/route/conftest.py rename to testsuite/tests/singlecluster/limitador/section/conftest.py index 98902700..38911259 100644 --- a/testsuite/tests/singlecluster/limitador/route/conftest.py +++ b/testsuite/tests/singlecluster/limitador/section/conftest.py @@ -1,4 +1,4 @@ -"""Conftest for RLP targeting route tests """ +"""Conftest for RLP section_name targeting tests""" import pytest @@ -18,3 +18,11 @@ def route(route, backend): RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX)), ) return route + + +@pytest.fixture(scope="module", autouse=True) +def commit(request, route, rate_limit): # pylint: disable=unused-argument + """Commits RateLimitPolicy after the HTTPRoute is created""" + request.addfinalizer(rate_limit.delete) + rate_limit.commit() + rate_limit.wait_for_ready() diff --git a/testsuite/tests/singlecluster/limitador/section/test_listener.py b/testsuite/tests/singlecluster/limitador/section/test_listener.py new file mode 100644 index 00000000..9c343230 --- /dev/null +++ b/testsuite/tests/singlecluster/limitador/section/test_listener.py @@ -0,0 +1,24 @@ +"""Tests that the RLP is correctly applies to the Gateway Listener.""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy + +pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] + + +@pytest.fixture(scope="module") +def rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy targeting the Gateway Listener.""" + rlp = RateLimitPolicy.create_instance(cluster, blame("limit"), gateway, "api", labels={"testRun": module_label}) + rlp.add_limit("basic", [Limit(5, "10s")]) + return rlp + + +def test_listener_match(client): + """Tests that RLP correctly applies to the given Gateway Listener""" + responses = client.get_many("/get", 5) + responses.assert_all(status_code=200) + + assert client.get("/get").status_code == 429 + assert client.get("/anything").status_code == 429 diff --git a/testsuite/tests/singlecluster/limitador/section/test_multiple_rules.py b/testsuite/tests/singlecluster/limitador/section/test_multiple_rules.py new file mode 100644 index 00000000..fa599737 --- /dev/null +++ b/testsuite/tests/singlecluster/limitador/section/test_multiple_rules.py @@ -0,0 +1,51 @@ +"""Test multiple RLP's targeting different HTTPRouteRules do not interfere with each other.""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy + + +pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] + + +@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("limit"), route, "rule-1", labels={"testRun": module_label} + ) + rate_limit.add_limit("basic", [Limit(3, "5s")]) + return rate_limit + + +@pytest.fixture(scope="module") +def rate_limit2(cluster, blame, module_label, route): + """Add a RateLimitPolicy targeting the second HTTPRouteRule.""" + rlp = RateLimitPolicy.create_instance(cluster, blame("limit"), route, "rule-2", labels={"testRun": module_label}) + rlp.add_limit("basic", [Limit(2, "5s")]) + return rlp + + +@pytest.fixture(scope="module", autouse=True) +def commit(request, rate_limit, rate_limit2): + """Commit and wait for RateLimitPolicies to be fully enforced""" + for policy in [rate_limit, rate_limit2]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_ready() + + +def test_multiple_limits_targeting_rules(client): + """Test targeting separate HTTPRouteRules with different limits""" + responses = client.get_many("/get", 3) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + + responses = client.get_many("/anything", 2) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + + assert client.get("/get").status_code == 429 + assert client.get("/anything").status_code == 429 diff --git a/testsuite/tests/singlecluster/limitador/section/test_multiple_same_listener.py b/testsuite/tests/singlecluster/limitador/section/test_multiple_same_listener.py new file mode 100644 index 00000000..8eb95f11 --- /dev/null +++ b/testsuite/tests/singlecluster/limitador/section/test_multiple_same_listener.py @@ -0,0 +1,28 @@ +"""Test that multiple limits targeting same Gateway Listener are correctly applied""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy + + +pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] + + +@pytest.fixture(scope="module") +def rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy targeting the Gateway Listener with multiple limits.""" + rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), gateway, "api", labels={"testRun": module_label} + ) + rate_limit.add_limit("test1", [Limit(8, "10s")]) + rate_limit.add_limit("test2", [Limit(3, "5s")]) + return rate_limit + + +def test_two_limits_targeting_one_listener(client): + """Test that one limit ends up shadowing others""" + responses = client.get_many("/get", 3) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + assert client.get("/get").status_code == 429 diff --git a/testsuite/tests/singlecluster/limitador/route/test_multiple_same_rule.py b/testsuite/tests/singlecluster/limitador/section/test_multiple_same_rule.py similarity index 50% rename from testsuite/tests/singlecluster/limitador/route/test_multiple_same_rule.py rename to testsuite/tests/singlecluster/limitador/section/test_multiple_same_rule.py index 0796e1bb..23d07ed4 100644 --- a/testsuite/tests/singlecluster/limitador/route/test_multiple_same_rule.py +++ b/testsuite/tests/singlecluster/limitador/section/test_multiple_same_rule.py @@ -2,24 +2,24 @@ import pytest -from testsuite.kuadrant.policy import CelPredicate -from testsuite.kuadrant.policy.rate_limit import Limit +from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] @pytest.fixture(scope="module") -def rate_limit(rate_limit): - """Add limit to the policy""" - when = CelPredicate("request.path == '/get'") - rate_limit.add_limit("test1", [Limit(8, "10s")], when=[when]) - rate_limit.add_limit("test2", [Limit(3, "5s")], when=[when]) +def rate_limit(cluster, blame, module_label, route): + """Add a RateLimitPolicy targeting the first HTTPRouteRule with two limits.""" + rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), route, "rule-1", labels={"testRun": module_label} + ) + rate_limit.add_limit("test1", [Limit(8, "10s")]) + rate_limit.add_limit("test2", [Limit(3, "5s")]) return rate_limit -@pytest.mark.issue("https://github.com/Kuadrant/testsuite/issues/561") -def test_two_rules_targeting_one_limit(client): +def test_two_limits_targeting_one_rule(client): """Test that one limit ends up shadowing others""" responses = client.get_many("/get", 3) assert all( diff --git a/testsuite/tests/singlecluster/limitador/section/test_route_rule.py b/testsuite/tests/singlecluster/limitador/section/test_route_rule.py new file mode 100644 index 00000000..3a8d489e --- /dev/null +++ b/testsuite/tests/singlecluster/limitador/section/test_route_rule.py @@ -0,0 +1,26 @@ +"""Tests that the RLP is correctly applied to the route rule""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy + +pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] + + +@pytest.fixture(scope="module") +def rate_limit(cluster, blame, module_label, route): + """Add a RateLimitPolicy targeting the first HTTPRouteRule.""" + rlp = RateLimitPolicy.create_instance(cluster, blame("limit"), route, "rule-1", labels={"testRun": module_label}) + rlp.add_limit("basic", [Limit(5, "10s")]) + return rlp + + +def test_rule_match(client): + """Tests that RLP correctly applies to the given HTTPRouteRule""" + responses = client.get_many("/get", 5) + responses.assert_all(status_code=200) + + assert client.get("/get").status_code == 429 + + response = client.get("/anything") + assert response.status_code == 200 From 85510e8eb0f52b3dc8ba5e88bbc9dbb115e636bd Mon Sep 17 00:00:00 2001 From: Martin Hesko Date: Tue, 26 Nov 2024 12:07:59 +0100 Subject: [PATCH 3/8] Expand D&O tests: sectionName/Multiple policies with one target Signed-off-by: Martin Hesko --- .../defaults/section/__init__.py | 0 .../defaults/section/conftest.py | 12 +++++ .../defaults/section/test_basic_listener.py | 48 +++++++++++++++++++ .../defaults/section/test_basic_route_rule.py | 48 +++++++++++++++++++ .../overrides/section/__init__.py | 0 .../overrides/section/conftest.py | 12 +++++ .../section/test_listener_multiple.py | 39 +++++++++++++++ .../overrides/section/test_rule_multiple.py | 39 +++++++++++++++ .../overrides/test_gateway_multiple.py | 46 ++++++++++++++++++ .../overrides/test_route_multiple.py | 46 ++++++++++++++++++ 10 files changed, 290 insertions(+) create mode 100644 testsuite/tests/singlecluster/defaults/section/__init__.py create mode 100644 testsuite/tests/singlecluster/defaults/section/conftest.py create mode 100644 testsuite/tests/singlecluster/defaults/section/test_basic_listener.py create mode 100644 testsuite/tests/singlecluster/defaults/section/test_basic_route_rule.py create mode 100644 testsuite/tests/singlecluster/overrides/section/__init__.py create mode 100644 testsuite/tests/singlecluster/overrides/section/conftest.py create mode 100644 testsuite/tests/singlecluster/overrides/section/test_listener_multiple.py create mode 100644 testsuite/tests/singlecluster/overrides/section/test_rule_multiple.py create mode 100644 testsuite/tests/singlecluster/overrides/test_gateway_multiple.py create mode 100644 testsuite/tests/singlecluster/overrides/test_route_multiple.py diff --git a/testsuite/tests/singlecluster/defaults/section/__init__.py b/testsuite/tests/singlecluster/defaults/section/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testsuite/tests/singlecluster/defaults/section/conftest.py b/testsuite/tests/singlecluster/defaults/section/conftest.py new file mode 100644 index 00000000..08914737 --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/section/conftest.py @@ -0,0 +1,12 @@ +"""Conftest for RLP section_name targeting tests""" + +import pytest + + +@pytest.fixture(scope="module", autouse=True) +def commit(request, route, rate_limit, authorization): # pylint: disable=unused-argument + """Commits RateLimitPolicy after the HTTPRoute is created""" + for policy in [authorization, rate_limit]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_ready() diff --git a/testsuite/tests/singlecluster/defaults/section/test_basic_listener.py b/testsuite/tests/singlecluster/defaults/section/test_basic_listener.py new file mode 100644 index 00000000..2e8b4ed6 --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/section/test_basic_listener.py @@ -0,0 +1,48 @@ +"""Test enforcement of policies with defaults targeting a Gateway Listener""" + +import pytest + +from testsuite.httpx.auth import HttpxOidcClientAuth +from testsuite.kuadrant.policy.authorization.auth_policy import AuthPolicy +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit + +pytestmark = [pytest.mark.kuadrant_only] + +LIMIT = Limit(10, "10s") + + +@pytest.fixture(scope="module") +def authorization(cluster, blame, module_label, gateway, oidc_provider): + """Add oidc identity to defaults block of AuthPolicy""" + authorization = AuthPolicy.create_instance( + cluster, blame("authz"), gateway, labels={"testRun": module_label}, section_name="api" + ) + authorization.defaults.identity.add_oidc("default", oidc_provider.well_known["issuer"]) + return authorization + + +@pytest.fixture(scope="module") +def auth(oidc_provider): + """Returns Authentication object for HTTPX""" + return HttpxOidcClientAuth(oidc_provider.get_token, "authorization") + + +@pytest.fixture(scope="module") +def rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy targeting Gateway Listener.""" + rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), gateway, "api", labels={"testRun": module_label} + ) + rate_limit.defaults.add_limit("basic", [LIMIT]) + return rate_limit + + +def test_basic_listener(client, auth): + """Test the defaults policies are correctly applied to the Gateway's Listener.""" + assert client.get("/get").status_code == 401 + + responses = client.get_many("/get", LIMIT.limit - 1, auth=auth) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + assert client.get("/get", auth=auth).status_code == 429 diff --git a/testsuite/tests/singlecluster/defaults/section/test_basic_route_rule.py b/testsuite/tests/singlecluster/defaults/section/test_basic_route_rule.py new file mode 100644 index 00000000..9c2104f7 --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/section/test_basic_route_rule.py @@ -0,0 +1,48 @@ +"""Test enforcement of policies with defaults targeting a specific HTTPRouteRule""" + +import pytest + +from testsuite.httpx.auth import HttpxOidcClientAuth +from testsuite.kuadrant.policy.authorization.auth_policy import AuthPolicy +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit + +pytestmark = [pytest.mark.kuadrant_only] + +LIMIT = Limit(10, "10s") + + +@pytest.fixture(scope="module") +def authorization(cluster, blame, module_label, route, oidc_provider): + """Add oidc identity targeting the first HTTPRouteRule""" + authorization = AuthPolicy.create_instance( + cluster, blame("authz"), route, labels={"testRun": module_label}, section_name="rule-1" + ) + authorization.defaults.identity.add_oidc("default", oidc_provider.well_known["issuer"]) + return authorization + + +@pytest.fixture(scope="module") +def auth(oidc_provider): + """Returns Authentication object for HTTPX""" + return HttpxOidcClientAuth(oidc_provider.get_token, "authorization") + + +@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("limit"), route, "rule-1", labels={"testRun": module_label} + ) + rate_limit.defaults.add_limit("basic", [LIMIT]) + return rate_limit + + +def test_basic_authorization(client, auth): + """Test the defaults policies are correctly applied to the HTTPRouteRule.""" + assert client.get("/get").status_code == 401 + + responses = client.get_many("/get", LIMIT.limit - 1, auth=auth) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + assert client.get("/get", auth=auth).status_code == 429 diff --git a/testsuite/tests/singlecluster/overrides/section/__init__.py b/testsuite/tests/singlecluster/overrides/section/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testsuite/tests/singlecluster/overrides/section/conftest.py b/testsuite/tests/singlecluster/overrides/section/conftest.py new file mode 100644 index 00000000..cce94b60 --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/section/conftest.py @@ -0,0 +1,12 @@ +"""Conftest for overrides section_name targeting tests.""" + +import pytest + + +@pytest.fixture(scope="module", autouse=True) +def commit(request, route, rate_limit, override_rate_limit): # pylint: disable=unused-argument + """Commits RateLimitPolicy after the HTTPRoute is created""" + for policy in [rate_limit, override_rate_limit]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_ready() diff --git a/testsuite/tests/singlecluster/overrides/section/test_listener_multiple.py b/testsuite/tests/singlecluster/overrides/section/test_listener_multiple.py new file mode 100644 index 00000000..0508fca0 --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/section/test_listener_multiple.py @@ -0,0 +1,39 @@ +"""Test override overriding another policy aimed at the same Gateway Listener.""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit + +pytestmark = [pytest.mark.kuadrant_only] + +LIMIT = Limit(8, "5s") +OVERRIDE_LIMIT = Limit(6, "5s") + + +@pytest.fixture(scope="module") +def rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy targeting the first HTTPRouteRule.""" + rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), gateway, "api", labels={"testRun": module_label} + ) + rate_limit.defaults.add_limit("basic", [LIMIT]) + return rate_limit + + +@pytest.fixture(scope="module") +def override_rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy targeting the first HTTPRouteRule.""" + override_rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), gateway, "api", labels={"testRun": module_label} + ) + override_rate_limit.overrides.add_limit("override", [OVERRIDE_LIMIT]) + return override_rate_limit + + +def test_multiple_policies_listener_override(client): + """Test RateLimitPolicy with an override overriding a default policy targeting the same Gateway Listener""" + responses = client.get_many("/get", OVERRIDE_LIMIT.limit) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + assert client.get("/get").status_code == 429 diff --git a/testsuite/tests/singlecluster/overrides/section/test_rule_multiple.py b/testsuite/tests/singlecluster/overrides/section/test_rule_multiple.py new file mode 100644 index 00000000..c7e9349e --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/section/test_rule_multiple.py @@ -0,0 +1,39 @@ +"""Test override overriding another policy aimed at the same Gateway Listener.""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit + +pytestmark = [pytest.mark.kuadrant_only] + +LIMIT = Limit(8, "5s") +OVERRIDE_LIMIT = Limit(6, "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("limit"), route, "rule-1", labels={"testRun": module_label} + ) + rate_limit.defaults.add_limit("basic", [LIMIT]) + return rate_limit + + +@pytest.fixture(scope="module") +def override_rate_limit(cluster, blame, module_label, route): + """Add a RateLimitPolicy targeting the first HTTPRouteRule.""" + override_rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), route, "rule-1", labels={"testRun": module_label} + ) + override_rate_limit.overrides.add_limit("override", [OVERRIDE_LIMIT]) + return override_rate_limit + + +def test_multiple_policies_rule_override(client): + """Test RateLimitPolicy with an override overriding a default policy targeting the same HTTPRouteRule""" + responses = client.get_many("/get", OVERRIDE_LIMIT.limit) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + assert client.get("/get").status_code == 429 diff --git a/testsuite/tests/singlecluster/overrides/test_gateway_multiple.py b/testsuite/tests/singlecluster/overrides/test_gateway_multiple.py new file mode 100644 index 00000000..fc4a3833 --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/test_gateway_multiple.py @@ -0,0 +1,46 @@ +"""Test override overriding another policy aimed at the same Gateway.""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit + +pytestmark = [pytest.mark.kuadrant_only] + +LIMIT = Limit(10, "10s") +OVERRIDE_LIMIT = Limit(5, "10s") + + +@pytest.fixture(scope="module") +def rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy with a default limit targeting the Gateway.""" + rate_limit = RateLimitPolicy.create_instance(cluster, blame("limit"), gateway, labels={"testRun": module_label}) + rate_limit.defaults.add_limit("basic", [LIMIT]) + return rate_limit + + +@pytest.fixture(scope="module") +def override_rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy with an overrride targeting the Gateway.""" + override_rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), gateway, labels={"testRun": module_label} + ) + override_rate_limit.overrides.add_limit("override", [OVERRIDE_LIMIT]) + return override_rate_limit + + +@pytest.fixture(scope="module", autouse=True) +def commit(request, route, rate_limit, override_rate_limit): # pylint: disable=unused-argument + """Commits RateLimitPolicies after the HTTPRoute is created and checks correct status""" + for policy in [rate_limit, override_rate_limit]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_ready() + + +def test_multiple_policies_gateway_override(client): + """Test RateLimitPolicy with an override overriding a default policy targeting the same Gateway.""" + responses = client.get_many("/get", OVERRIDE_LIMIT.limit) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + assert client.get("/get").status_code == 429 diff --git a/testsuite/tests/singlecluster/overrides/test_route_multiple.py b/testsuite/tests/singlecluster/overrides/test_route_multiple.py new file mode 100644 index 00000000..ff163065 --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/test_route_multiple.py @@ -0,0 +1,46 @@ +"""Test override overriding another policy targeting the same HTTPRoute.""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit + +pytestmark = [pytest.mark.kuadrant_only] + +LIMIT = Limit(10, "10s") +OVERRIDE_LIMIT = Limit(5, "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("limit"), route, labels={"testRun": module_label}) + rate_limit.defaults.add_limit("basic", [LIMIT]) + return rate_limit + + +@pytest.fixture(scope="module") +def override_rate_limit(cluster, blame, module_label, route): + """Add a RateLimitPolicy targeting the first HTTPRouteRule.""" + override_rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), route, labels={"testRun": module_label} + ) + override_rate_limit.overrides.add_limit("override", [OVERRIDE_LIMIT]) + return override_rate_limit + + +@pytest.fixture(scope="module", autouse=True) +def commit(request, rate_limit, override_rate_limit): # pylint: disable=unused-argument + """Commits RateLimitPolicy after the HTTPRoute is created""" + for policy in [rate_limit, override_rate_limit]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_ready() + + +def test_multiple_policies_route_override(client): + """Test RateLimitPolicy with an override overriding a default policy targeting the same HTTPRoute""" + responses = client.get_many("/get", OVERRIDE_LIMIT.limit) + assert all( + r.status_code == 200 for r in responses + ), f"Rate Limited resource unexpectedly rejected requests {responses}" + assert client.get("/get").status_code == 429 From dcf866816611851bd9a9096c410983b7020f8dd6 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Tue, 26 Nov 2024 11:22:14 +0000 Subject: [PATCH 4/8] ADD: strategy to policy Allow adding strategy field to policies. Signed-off-by: Jim Fitzpatrick --- testsuite/kuadrant/policy/__init__.py | 8 ++++++++ .../policy/authorization/auth_policy.py | 20 +++++++++++++++++-- testsuite/kuadrant/policy/rate_limit.py | 17 +++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/testsuite/kuadrant/policy/__init__.py b/testsuite/kuadrant/policy/__init__.py index d4ba2ad8..2666af52 100644 --- a/testsuite/kuadrant/policy/__init__.py +++ b/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'""" diff --git a/testsuite/kuadrant/policy/authorization/auth_policy.py b/testsuite/kuadrant/policy/authorization/auth_policy.py index 62d2292f..4830a2fb 100644 --- a/testsuite/kuadrant/policy/authorization/auth_policy.py +++ b/testsuite/kuadrant/policy/authorization/auth_policy.py @@ -7,10 +7,11 @@ from testsuite.kubernetes import modify from testsuite.kubernetes.client import KubernetesClient from testsuite.utils import asdict + +from .. import CelPredicate, Policy, Strategy +from . import Pattern from .auth_config import AuthConfig from .sections import ResponseSection -from .. import Policy, CelPredicate -from . import Pattern class AuthPolicy(Policy, AuthConfig): @@ -49,6 +50,21 @@ def add_rule(self, when: list[CelPredicate]): self.model.spec.setdefault("when", []) self.model.spec["when"].extend([asdict(x) for x in when]) + # TODO: need to check if the typing is set up correctlly. + @modify + def strategy(self, strategy: Strategy) -> None: + """Add strategy type to default or overrides spec""" + if self.spec_section is None: + if "defaults" in self.model.spec: + self.spec_section = self.model.spec["default"] + elif "overrides" in self.model.spec: + self.spec_section = self.model.spec["overrides"] + else: + raise TypeError("no default or override section found in spec") + + self.spec_section["strategy"] = strategy.value + self.spec_section = None + @property def auth_section(self): if self.spec_section is None: diff --git a/testsuite/kuadrant/policy/rate_limit.py b/testsuite/kuadrant/policy/rate_limit.py index 7332f751..95fd48cc 100644 --- a/testsuite/kuadrant/policy/rate_limit.py +++ b/testsuite/kuadrant/policy/rate_limit.py @@ -5,9 +5,9 @@ from typing import Iterable from testsuite.gateway import Referencable +from testsuite.kuadrant.policy import CelExpression, CelPredicate, Policy, Strategy from testsuite.kubernetes import modify from testsuite.kubernetes.client import KubernetesClient -from testsuite.kuadrant.policy import Policy, CelPredicate, CelExpression from testsuite.utils import asdict @@ -72,6 +72,21 @@ def add_limit( self.spec_section.setdefault("limits", {})[name] = limit self.spec_section = None + # TODO: need to check if the typing is set up correctlly. + @modify + def strategy(self, strategy: Strategy) -> None: + """Add strategy type to default or overrides spec""" + if self.spec_section is None: + if "defaults" in self.model.spec: + self.spec_section = self.model.spec["defaults"] + elif "overrides" in self.model.spec: + self.spec_section = self.model.spec["overrides"] + else: + raise TypeError("no default or override section found in spec") + + self.spec_section["strategy"] = strategy.value + self.spec_section = None + @property def defaults(self): """Add new rule into the `defaults` RateLimitPolicy section""" From 6962e2ee9afb87bcfd0261e7b30c8e21e3ccf028 Mon Sep 17 00:00:00 2001 From: Martin Hesko Date: Fri, 29 Nov 2024 12:46:15 +0100 Subject: [PATCH 5/8] merge strategy tests for D&O Signed-off-by: Martin Hesko --- .../singlecluster/defaults/merge/__init__.py | 0 .../singlecluster/defaults/merge/conftest.py | 20 ++++++++ .../merge/test_gateway_default_merge.py | 51 +++++++++++++++++++ .../singlecluster/overrides/merge/__init__.py | 0 .../singlecluster/overrides/merge/conftest.py | 20 ++++++++ .../merge/test_gateway_override_merge.py | 51 +++++++++++++++++++ 6 files changed, 142 insertions(+) create mode 100644 testsuite/tests/singlecluster/defaults/merge/__init__.py create mode 100644 testsuite/tests/singlecluster/defaults/merge/conftest.py create mode 100644 testsuite/tests/singlecluster/defaults/merge/test_gateway_default_merge.py create mode 100644 testsuite/tests/singlecluster/overrides/merge/__init__.py create mode 100644 testsuite/tests/singlecluster/overrides/merge/conftest.py create mode 100644 testsuite/tests/singlecluster/overrides/merge/test_gateway_override_merge.py diff --git a/testsuite/tests/singlecluster/defaults/merge/__init__.py b/testsuite/tests/singlecluster/defaults/merge/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testsuite/tests/singlecluster/defaults/merge/conftest.py b/testsuite/tests/singlecluster/defaults/merge/conftest.py new file mode 100644 index 00000000..153964bc --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/merge/conftest.py @@ -0,0 +1,20 @@ +"""Conftest for merge strategy tests""" + +import pytest + +from testsuite.gateway import RouteMatch, PathMatch, MatchType + + +@pytest.fixture(scope="module") +def route(route, backend): + """Add two new rules to the route""" + route.remove_all_rules() + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX)), + ) + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX)), + ) + return route diff --git a/testsuite/tests/singlecluster/defaults/merge/test_gateway_default_merge.py b/testsuite/tests/singlecluster/defaults/merge/test_gateway_default_merge.py new file mode 100644 index 00000000..b363a5f7 --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/merge/test_gateway_default_merge.py @@ -0,0 +1,51 @@ +"""Test gateway level default merging with and being patrially overriden by another policy.""" + +import pytest + +from testsuite.kuadrant.policy import CelPredicate +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit, Strategy + +pytestmark = [pytest.mark.kuadrant_only] + + +@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(3, "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): + """Test Gateway default policy being partially overriden when another policy with the same target is created.""" + get = client.get_many("/get", 3) + 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 diff --git a/testsuite/tests/singlecluster/overrides/merge/__init__.py b/testsuite/tests/singlecluster/overrides/merge/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testsuite/tests/singlecluster/overrides/merge/conftest.py b/testsuite/tests/singlecluster/overrides/merge/conftest.py new file mode 100644 index 00000000..153964bc --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/merge/conftest.py @@ -0,0 +1,20 @@ +"""Conftest for merge strategy tests""" + +import pytest + +from testsuite.gateway import RouteMatch, PathMatch, MatchType + + +@pytest.fixture(scope="module") +def route(route, backend): + """Add two new rules to the route""" + route.remove_all_rules() + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX)), + ) + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX)), + ) + return route diff --git a/testsuite/tests/singlecluster/overrides/merge/test_gateway_override_merge.py b/testsuite/tests/singlecluster/overrides/merge/test_gateway_override_merge.py new file mode 100644 index 00000000..b0aa8bb4 --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/merge/test_gateway_override_merge.py @@ -0,0 +1,51 @@ +"""Test override overriding another policy aimed at the same Gateway Listener.""" + +import pytest + +from testsuite.kuadrant.policy import CelPredicate +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit, Strategy + +pytestmark = [pytest.mark.kuadrant_only] + + +@pytest.fixture(scope="function") +def override_rate_limit(cluster, blame, module_label, gateway): + """Add a RateLimitPolicy with a merge strategy override targeting a specific endpoint.""" + override_rate_limit = RateLimitPolicy.create_instance( + cluster, blame("limit"), gateway, labels={"testRun": module_label} + ) + when = CelPredicate("request.path == '/get'") + override_rate_limit.overrides.add_limit("route_limit", [Limit(3, "5s")], when=[when]) + override_rate_limit.overrides.strategy(Strategy.MERGE) + return override_rate_limit + + +@pytest.fixture(scope="module") +def rate_limit(rate_limit): + """Add limits targeted at specific endpoints to the RateLimitPolicy.""" + gateway_when = CelPredicate("request.path == '/anything'") + rate_limit.add_limit("gateway_limit", [Limit(3, "5s")], when=[gateway_when]) + route_when = CelPredicate("request.path == '/get'") + rate_limit.add_limit("route_limit", [Limit(10, "5s")], when=[route_when]) + return rate_limit + + +@pytest.fixture(scope="function", autouse=True) +def commit(request, route, rate_limit, override_rate_limit): # pylint: disable=unused-argument + """Commits RateLimitPolicy after the HTTPRoute is created""" + for policy in [override_rate_limit, rate_limit]: # Forcing order of creation. + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_accepted() + + +@pytest.mark.parametrize("rate_limit", ["gateway", "route"], indirect=True) +def test_gateway_override_merge(client): + """Test RateLimitPolicy with an override and merge strategy overriding only a part of a new policy.""" + get = client.get_many("/get", 3) + 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 From a8f9a590285ff662fc79ee1eb2358fd0dfe8e3a0 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Tue, 3 Dec 2024 09:25:01 +0000 Subject: [PATCH 6/8] ADD: Defaults and overrides: default merge on same target This adds test that make use of the merge strategy on defaults that are targeting the same resource. Signed-off-by: Jim Fitzpatrick --- .../defaults/merge/same_target/__init__.py | 0 .../defaults/merge/same_target/conftest.py | 51 +++++++++++++++++++ .../merge/same_target/test_ab_strategy.py | 28 ++++++++++ .../merge/same_target/test_ba_startegy.py | 28 ++++++++++ 4 files changed, 107 insertions(+) create mode 100644 testsuite/tests/singlecluster/defaults/merge/same_target/__init__.py create mode 100644 testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py create mode 100644 testsuite/tests/singlecluster/defaults/merge/same_target/test_ab_strategy.py create mode 100644 testsuite/tests/singlecluster/defaults/merge/same_target/test_ba_startegy.py diff --git a/testsuite/tests/singlecluster/defaults/merge/same_target/__init__.py b/testsuite/tests/singlecluster/defaults/merge/same_target/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py b/testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py new file mode 100644 index 00000000..0042e36e --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py @@ -0,0 +1,51 @@ +import pytest + +from testsuite.gateway import MatchType, PathMatch, RouteMatch +from testsuite.kuadrant.policy import CelPredicate, Strategy +from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy + +LIMIT = Limit(8, "10s") +MERGE_LIMIT = Limit(6, "10s") +NO_LIMIT = Limit(10, "10s") + + +@pytest.fixture(scope="module", autouse=True) +def route(route, backend): + """Add two new rules to the route""" + route.remove_all_rules() + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX)), + ) + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX)), + ) + return route + + +@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}) + basic_when = [ + CelPredicate("request.path == '/get'"), + ] + rate_limit.add_limit("basic", [LIMIT], when=basic_when) + 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}) + basic_when = [ + CelPredicate("request.path == '/get'"), + ] + merge_when = [ + CelPredicate("request.path == '/anything'"), + ] + policy.defaults.add_limit("basic", [MERGE_LIMIT], when=basic_when) + policy.defaults.add_limit("merge", [MERGE_LIMIT], when=merge_when) + policy.strategy(Strategy.MERGE) + return policy diff --git a/testsuite/tests/singlecluster/defaults/merge/same_target/test_ab_strategy.py b/testsuite/tests/singlecluster/defaults/merge/same_target/test_ab_strategy.py new file mode 100644 index 00000000..a66e9e47 --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/merge/same_target/test_ab_strategy.py @@ -0,0 +1,28 @@ +"""Test defaults policy aimed at the same resoure uses oldested policy.""" + +import pytest + +from .conftest import MERGE_LIMIT + +pytestmark = [pytest.mark.kuadrant_only] + + +@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]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_accepted() + + +@pytest.mark.parametrize("rate_limit", ["gateway", "route"], indirect=True) +def test_multiple_policies_merge_default_ab(client): + """Test RateLimitPolicy with merge defaults being ingored due to age""" + responses = client.get_many("/get", MERGE_LIMIT.limit) + responses.assert_all(200) + assert client.get("/get").status_code == 429 + + responses = client.get_many("/anything", MERGE_LIMIT.limit) + responses.assert_all(200) + assert client.get("/anything").status_code == 429 diff --git a/testsuite/tests/singlecluster/defaults/merge/same_target/test_ba_startegy.py b/testsuite/tests/singlecluster/defaults/merge/same_target/test_ba_startegy.py new file mode 100644 index 00000000..a451a38a --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/merge/same_target/test_ba_startegy.py @@ -0,0 +1,28 @@ +"""Test defaults policy aimed at the same resoure uses oldested policy.""" + +import pytest + +from .conftest import LIMIT, MERGE_LIMIT + +pytestmark = [pytest.mark.kuadrant_only] + + +@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]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_accepted() + + +@pytest.mark.parametrize("rate_limit", ["gateway", "route"], indirect=True) +def test_multiple_policies_merge_default_ba(client): + """Test RateLimitPolicy with merge defaults being enforced due to age""" + responses = client.get_many("/get", LIMIT.limit) + responses.assert_all(200) + assert client.get("/get").status_code == 429 + + responses = client.get_many("/anything", MERGE_LIMIT.limit) + responses.assert_all(200) + assert client.get("/anything").status_code == 429 From 0852e073e8760af44b6fcb63ec02c5b09bc2d8d7 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Tue, 3 Dec 2024 11:54:35 +0000 Subject: [PATCH 7/8] ADD: Defaults and overrides: override merge on same target This adds test that make use of the merge strategy on overrides that are targeting the same resource. Signed-off-by: Jim Fitzpatrick --- .../overrides/merge/same_target/__init__.py | 0 .../overrides/merge/same_target/conftest.py | 50 +++++++++++++++++++ .../merge/same_target/test_ab_strategy.py | 28 +++++++++++ .../merge/same_target/test_ba_startegy.py | 28 +++++++++++ 4 files changed, 106 insertions(+) create mode 100644 testsuite/tests/singlecluster/overrides/merge/same_target/__init__.py create mode 100644 testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py create mode 100644 testsuite/tests/singlecluster/overrides/merge/same_target/test_ab_strategy.py create mode 100644 testsuite/tests/singlecluster/overrides/merge/same_target/test_ba_startegy.py diff --git a/testsuite/tests/singlecluster/overrides/merge/same_target/__init__.py b/testsuite/tests/singlecluster/overrides/merge/same_target/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py b/testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py new file mode 100644 index 00000000..328c42a3 --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py @@ -0,0 +1,50 @@ +import pytest + +from testsuite.gateway import MatchType, PathMatch, RouteMatch +from testsuite.kuadrant.policy import CelPredicate, Strategy +from testsuite.kuadrant.policy.rate_limit import Limit, RateLimitPolicy + +LIMIT = Limit(8, "5s") +OVERRIDE_LIMIT = Limit(6, "5s") + + +@pytest.fixture(scope="module") +def route(route, backend): + """Add two new rules to the route""" + route.remove_all_rules() + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX)), + ) + route.add_rule( + backend, + RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX)), + ) + return route + + +@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}) + basic_when = [ + CelPredicate("request.path == '/get'"), + ] + rate_limit.add_limit("basic", [LIMIT], when=basic_when) + 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}) + basic_when = [ + CelPredicate("request.path == '/get'"), + ] + override_when = [ + CelPredicate("request.path == '/anything'"), + ] + policy.overrides.add_limit("basic", [OVERRIDE_LIMIT], when=basic_when) + policy.overrides.add_limit("override", [OVERRIDE_LIMIT], when=override_when) + policy.strategy(Strategy.MERGE) + return policy diff --git a/testsuite/tests/singlecluster/overrides/merge/same_target/test_ab_strategy.py b/testsuite/tests/singlecluster/overrides/merge/same_target/test_ab_strategy.py new file mode 100644 index 00000000..c158fed2 --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/merge/same_target/test_ab_strategy.py @@ -0,0 +1,28 @@ +"""Test override policies aimed at the same resoure uses oldested policy.""" + +import pytest + +from .conftest import OVERRIDE_LIMIT + +pytestmark = [pytest.mark.kuadrant_only] + + +@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]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_accepted() + + +@pytest.mark.parametrize("rate_limit", ["gateway", "route"], indirect=True) +def test_multiple_policies_merge_override_ab(client): + """Test RateLimitPolicy with merge overrides being ingored due to age""" + responses = client.get_many("/get", OVERRIDE_LIMIT.limit) + responses.assert_all(200) + assert client.get("/get").status_code == 429 + + responses = client.get_many("/anything", OVERRIDE_LIMIT.limit) + responses.assert_all(200) + assert client.get("/anything").status_code == 429 diff --git a/testsuite/tests/singlecluster/overrides/merge/same_target/test_ba_startegy.py b/testsuite/tests/singlecluster/overrides/merge/same_target/test_ba_startegy.py new file mode 100644 index 00000000..23234edb --- /dev/null +++ b/testsuite/tests/singlecluster/overrides/merge/same_target/test_ba_startegy.py @@ -0,0 +1,28 @@ +"""Test defaults policy aimed at the same resoure uses oldested policy.""" + +import pytest + +from .conftest import OVERRIDE_LIMIT + +pytestmark = [pytest.mark.kuadrant_only] + + +@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]: + request.addfinalizer(policy.delete) + policy.commit() + policy.wait_for_accepted() + + +@pytest.mark.parametrize("rate_limit", ["gateway", "route"], indirect=True) +def test_multiple_policies_merge_default_ba(client): + """Test RateLimitPolicy with merge overrides being enforced due to age""" + responses = client.get_many("/get", OVERRIDE_LIMIT.limit) + responses.assert_all(200) + assert client.get("/get").status_code == 429 + + responses = client.get_many("/anything", OVERRIDE_LIMIT.limit) + responses.assert_all(200) + assert client.get("/anything").status_code == 429 From 5095ef800a9288df7471d0d02b08830c7ba60545 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Mon, 9 Dec 2024 11:39:00 +0000 Subject: [PATCH 8/8] Update doc string Add missing doc string to conftest.py Signed-off-by: Jim Fitzpatrick --- .../tests/singlecluster/defaults/merge/same_target/conftest.py | 2 ++ .../tests/singlecluster/overrides/merge/same_target/conftest.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py b/testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py index 0042e36e..6b076806 100644 --- a/testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py +++ b/testsuite/tests/singlecluster/defaults/merge/same_target/conftest.py @@ -1,3 +1,5 @@ +"""Setup conftest for policy merge on the same targets""" + import pytest from testsuite.gateway import MatchType, PathMatch, RouteMatch diff --git a/testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py b/testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py index 328c42a3..ce9aef5f 100644 --- a/testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py +++ b/testsuite/tests/singlecluster/overrides/merge/same_target/conftest.py @@ -1,3 +1,5 @@ +"""Setup conftest for policy override on the same targets""" + import pytest from testsuite.gateway import MatchType, PathMatch, RouteMatch