From 489cc184ba2a73db78378de6310eb7d8c3da86a9 Mon Sep 17 00:00:00 2001 From: Martin Hesko Date: Wed, 20 Nov 2024 14:55:20 +0100 Subject: [PATCH 1/2] 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/2] 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