From 020a04efc7a00eb571d81179868291940f810aa4 Mon Sep 17 00:00:00 2001 From: Martin Hesko Date: Thu, 10 Oct 2024 17:23:09 +0200 Subject: [PATCH] Removed RouteSelectors due to version bump Signed-off-by: Martin Hesko --- testsuite/kuadrant/policy/rate_limit.py | 26 ++-------- .../method/test_route_subset_method.py | 10 ++-- .../route/test_limit_targeting_two_rules.py | 12 ++--- .../route/test_multiple_same_rule.py | 14 +++--- .../limitador/route/test_route_rule.py | 12 ++--- .../limitador/test_route_rule_invalid.py | 47 ------------------- 6 files changed, 25 insertions(+), 96 deletions(-) delete mode 100644 testsuite/tests/singlecluster/limitador/test_route_rule_invalid.py diff --git a/testsuite/kuadrant/policy/rate_limit.py b/testsuite/kuadrant/policy/rate_limit.py index d52c2ed7b..0dcadf8da 100644 --- a/testsuite/kuadrant/policy/rate_limit.py +++ b/testsuite/kuadrant/policy/rate_limit.py @@ -2,9 +2,9 @@ import time from dataclasses import dataclass -from typing import Iterable, Literal, Optional, List +from typing import Iterable, Literal -from testsuite.gateway import Referencable, RouteMatch +from testsuite.gateway import Referencable from testsuite.kubernetes import modify from testsuite.kubernetes.client import KubernetesClient from testsuite.kuadrant.policy import Policy @@ -21,23 +21,6 @@ class Limit: unit: Literal["second", "minute", "day"] = "second" -@dataclass -class RouteSelector: - """ - RouteSelector is an object composed of a set of HTTPRouteMatch objects (from Gateway API - - HTTPPathMatch, HTTPHeaderMatch, HTTPQueryParamMatch, HTTPMethodMatch), - and an additional hostnames field. - https://docs.kuadrant.io/kuadrant-operator/doc/reference/route-selectors/#routeselector - """ - - matches: Optional[list[RouteMatch]] = None - hostnames: Optional[list[str]] = None - - def __init__(self, *matches: RouteMatch, hostnames: Optional[List[str]] = None): - self.matches = list(matches) if matches else [] - self.hostnames = hostnames - - class RateLimitPolicy(Policy): """RateLimitPolicy (or RLP for short) object, used for applying rate limiting rules to a Gateway/HTTPRoute""" @@ -49,7 +32,7 @@ def __init__(self, *args, **kwargs): def create_instance(cls, cluster: KubernetesClient, name, target: Referencable, labels: dict[str, str] = None): """Creates new instance of RateLimitPolicy""" model = { - "apiVersion": "kuadrant.io/v1beta2", + "apiVersion": "kuadrant.io/v1beta3", "kind": "RateLimitPolicy", "metadata": {"name": name, "labels": labels}, "spec": { @@ -66,7 +49,6 @@ def add_limit( limits: Iterable[Limit], when: Iterable[Rule] = None, counters: list[str] = None, - route_selectors: Iterable[RouteSelector] = None, ): """Add another limit""" limit: dict = { @@ -76,8 +58,6 @@ def add_limit( limit["when"] = [asdict(rule) for rule in when] if counters: limit["counters"] = counters - if route_selectors: - limit["routeSelectors"] = [asdict(rule) for rule in route_selectors] if self.spec_section is None: self.spec_section = self.model.spec diff --git a/testsuite/tests/singlecluster/limitador/method/test_route_subset_method.py b/testsuite/tests/singlecluster/limitador/method/test_route_subset_method.py index 7141b197a..6157c5b2c 100644 --- a/testsuite/tests/singlecluster/limitador/method/test_route_subset_method.py +++ b/testsuite/tests/singlecluster/limitador/method/test_route_subset_method.py @@ -3,7 +3,8 @@ import pytest from testsuite.gateway import RouteMatch, PathMatch, MatchType, HTTPMethod -from testsuite.kuadrant.policy.rate_limit import Limit, RouteSelector +from testsuite.kuadrant.policy.authorization import Pattern +from testsuite.kuadrant.policy.rate_limit import Limit pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] @@ -27,13 +28,12 @@ def route(route, backend): @pytest.fixture(scope="module") def rate_limit(rate_limit): """Add limit to the policy""" - selector = RouteSelector( - RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX), method=HTTPMethod.GET) - ) - rate_limit.add_limit("anything", [Limit(5, 10)], route_selectors=[selector]) + when = [Pattern("request.path", "eq", "/get"), Pattern("request.method", "eq", "GET")] + rate_limit.add_limit("anything", [Limit(5, 10)], when=when) return rate_limit +@pytest.mark.issue("https://github.com/Kuadrant/kuadrant-operator/issues/821") def test_route_subset_method(client): """Tests that RLP for a HTTPRouteRule doesn't apply to separate HTTPRouteRule with different method""" responses = client.get_many("/anything", 5) 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 index 468f595ed..d9dfbd44a 100644 --- a/testsuite/tests/singlecluster/limitador/route/test_limit_targeting_two_rules.py +++ b/testsuite/tests/singlecluster/limitador/route/test_limit_targeting_two_rules.py @@ -2,8 +2,8 @@ import pytest -from testsuite.gateway import RouteMatch, PathMatch, MatchType -from testsuite.kuadrant.policy.rate_limit import RouteSelector, Limit +from testsuite.kuadrant.policy.authorization import Pattern +from testsuite.kuadrant.policy.rate_limit import Limit pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] @@ -12,14 +12,12 @@ @pytest.fixture(scope="module") def rate_limit(rate_limit): """Add limit to the policy""" - selector = RouteSelector( - RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX)), - RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX)), - ) - rate_limit.add_limit("test", [Limit(5, 10)], route_selectors=[selector]) + when = Pattern("request.method", "eq", "GET") + rate_limit.add_limit("test", [Limit(5, 10)], when=[when]) return rate_limit +@pytest.mark.issue("https://github.com/Kuadrant/kuadrant-operator/issues/821") def test_limit_targeting_two_rules(client): """Tests that one RLP limit targeting two rules limits them together""" responses = client.get_many("/get", 3) diff --git a/testsuite/tests/singlecluster/limitador/route/test_multiple_same_rule.py b/testsuite/tests/singlecluster/limitador/route/test_multiple_same_rule.py index 0b711d933..7e7060da7 100644 --- a/testsuite/tests/singlecluster/limitador/route/test_multiple_same_rule.py +++ b/testsuite/tests/singlecluster/limitador/route/test_multiple_same_rule.py @@ -2,8 +2,8 @@ import pytest -from testsuite.gateway import RouteMatch, PathMatch, MatchType -from testsuite.kuadrant.policy.rate_limit import RouteSelector, Limit +from testsuite.kuadrant.policy.rate_limit import Limit +from testsuite.kuadrant.policy.authorization import Pattern pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] @@ -12,14 +12,14 @@ @pytest.fixture(scope="module") def rate_limit(rate_limit): """Add limit to the policy""" - selector = RouteSelector( - RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX)), - ) - rate_limit.add_limit("test1", [Limit(8, 10)], route_selectors=[selector]) - rate_limit.add_limit("test2", [Limit(3, 5)], route_selectors=[selector]) + when = Pattern("request.path", "eq", "/get") + rate_limit.add_limit("test1", [Limit(8, 10)], when=[when]) + rate_limit.add_limit("test2", [Limit(3, 5)], when=[when]) return rate_limit +@pytest.mark.issue("https://github.com/Kuadrant/kuadrant-operator/issues/821") +@pytest.mark.issue("https://github.com/Kuadrant/wasm-shim/issues/104") def test_two_rules_targeting_one_limit(client): """Test that one limit ends up shadowing others""" responses = client.get_many("/get", 3) diff --git a/testsuite/tests/singlecluster/limitador/route/test_route_rule.py b/testsuite/tests/singlecluster/limitador/route/test_route_rule.py index 148a7e7a5..85c12d3d0 100644 --- a/testsuite/tests/singlecluster/limitador/route/test_route_rule.py +++ b/testsuite/tests/singlecluster/limitador/route/test_route_rule.py @@ -2,8 +2,8 @@ import pytest -from testsuite.gateway import RouteMatch, PathMatch, MatchType -from testsuite.kuadrant.policy.rate_limit import Limit, RouteSelector +from testsuite.kuadrant.policy.rate_limit import Limit +from testsuite.kuadrant.policy.authorization import Pattern pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] @@ -11,14 +11,12 @@ @pytest.fixture(scope="module") def rate_limit(rate_limit): """Add limit to the policy""" - selector = RouteSelector( - RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX)), - RouteMatch(path=PathMatch(value="/anything/test", type=MatchType.PATH_PREFIX)), - ) - rate_limit.add_limit("multiple", [Limit(5, 10)], route_selectors=[selector]) + when = [Pattern("request.path", "eq", "/get")] + rate_limit.add_limit("multiple", [Limit(5, 10)], when=when) return rate_limit +@pytest.mark.issue("https://github.com/Kuadrant/kuadrant-operator/issues/821") def test_rule_match(client): """Tests that RLP correctly applies to the given HTTPRoute rule""" responses = client.get_many("/get", 5) diff --git a/testsuite/tests/singlecluster/limitador/test_route_rule_invalid.py b/testsuite/tests/singlecluster/limitador/test_route_rule_invalid.py deleted file mode 100644 index 1968b2694..000000000 --- a/testsuite/tests/singlecluster/limitador/test_route_rule_invalid.py +++ /dev/null @@ -1,47 +0,0 @@ -"""Tests that RLP for a HTTPRouteRule doesn't take effect if there isn't a exact matching HTTPRouteRule""" - -import pytest - -from testsuite.gateway import RouteMatch, PathMatch, MatchType, HTTPMethod -from testsuite.kuadrant.policy.rate_limit import Limit, RouteSelector - - -pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] - - -@pytest.fixture(scope="module", params=["/anything/get", "/anything", "/get"]) -def endpoint(request): - """Endpoints to apply a RLP to""" - return request.param - - -@pytest.fixture(scope="module") -def route(route, backend): - """Add new rule to the route""" - route.remove_all_rules() - route.add_rule( - backend, - RouteMatch(path=PathMatch(value="/anything", type=MatchType.PATH_PREFIX), method=HTTPMethod.GET), - ) - route.add_rule( - backend, - RouteMatch(path=PathMatch(value="/get", type=MatchType.PATH_PREFIX), method=HTTPMethod.GET), - ) - return route - - -@pytest.fixture(scope="module") -def rate_limit(rate_limit, endpoint): - """Add limit to the policy""" - selector = RouteSelector(RouteMatch(path=PathMatch(value=endpoint, type=MatchType.EXACT), method=HTTPMethod.GET)) - rate_limit.add_limit("basic", [Limit(5, 10)], route_selectors=[selector]) - return rate_limit - - -def test_route_rule_invalid(client, endpoint): - """Tests that RLP for a HTTPRouteRule doesn't apply if there isn't an exact match""" - responses = client.get_many(endpoint, 5) - assert all( - r.status_code == 200 for r in responses - ), f"Rate Limited resource unexpectedly rejected requests {responses}" - assert client.get(endpoint).status_code == 200