From b387844be8b7db8b7ac3a3270ede7cb13a697f90 Mon Sep 17 00:00:00 2001 From: averevki Date: Tue, 30 Jul 2024 16:58:57 +0200 Subject: [PATCH 1/2] Add interface for defaults/overrides Signed-off-by: averevki --- .../policy/authorization/auth_policy.py | 28 ++++++++++++++++--- testsuite/kuadrant/policy/rate_limit.py | 24 ++++++++++++++-- testsuite/tests/singlecluster/conftest.py | 6 ++-- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/testsuite/kuadrant/policy/authorization/auth_policy.py b/testsuite/kuadrant/policy/authorization/auth_policy.py index 01d87bf0..58d691b5 100644 --- a/testsuite/kuadrant/policy/authorization/auth_policy.py +++ b/testsuite/kuadrant/policy/authorization/auth_policy.py @@ -16,9 +16,9 @@ class AuthPolicy(Policy, AuthConfig): """AuthPolicy object, it serves as Kuadrants AuthConfig""" - @property - def auth_section(self): - return self.model.spec.setdefault("rules", {}) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.spec_section = None @classmethod def create_instance( @@ -35,7 +35,6 @@ def create_instance( "metadata": {"name": name, "namespace": cluster.project, "labels": labels}, "spec": { "targetRef": target.reference, - "rules": {}, }, } @@ -46,3 +45,24 @@ def add_rule(self, when: list["Rule"]): """Add rule for the skip of entire AuthPolicy""" self.model.spec.setdefault("when", []) self.model.spec["when"].extend([asdict(x) for x in when]) + + @property + def auth_section(self): + if self.spec_section is None: + self.spec_section = self.model.spec + + spec_section = self.spec_section + self.spec_section = None + return spec_section.setdefault("rules", {}) + + @property + def defaults(self): + """Add new rule into the `defaults` AuthPolicy section""" + self.spec_section = self.model.spec.setdefault("defaults", {}) + return self + + @property + def overrides(self): + """Add new rule into the `overrides` AuthPolicy section""" + self.spec_section = self.model.spec.setdefault("overrides", {}) + return self diff --git a/testsuite/kuadrant/policy/rate_limit.py b/testsuite/kuadrant/policy/rate_limit.py index 48f6d0c3..d52c2ed7 100644 --- a/testsuite/kuadrant/policy/rate_limit.py +++ b/testsuite/kuadrant/policy/rate_limit.py @@ -41,6 +41,10 @@ def __init__(self, *matches: RouteMatch, hostnames: Optional[List[str]] = None): class RateLimitPolicy(Policy): """RateLimitPolicy (or RLP for short) object, used for applying rate limiting rules to a Gateway/HTTPRoute""" + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.spec_section = None + @classmethod def create_instance(cls, cluster: KubernetesClient, name, target: Referencable, labels: dict[str, str] = None): """Creates new instance of RateLimitPolicy""" @@ -50,7 +54,6 @@ def create_instance(cls, cluster: KubernetesClient, name, target: Referencable, "metadata": {"name": name, "labels": labels}, "spec": { "targetRef": target.reference, - "limits": {}, }, } @@ -75,7 +78,24 @@ def add_limit( limit["counters"] = counters if route_selectors: limit["routeSelectors"] = [asdict(rule) for rule in route_selectors] - self.model.spec.limits[name] = limit + + if self.spec_section is None: + self.spec_section = self.model.spec + + self.spec_section.setdefault("limits", {})[name] = limit + self.spec_section = None + + @property + def defaults(self): + """Add new rule into the `defaults` RateLimitPolicy section""" + self.spec_section = self.model.spec.setdefault("defaults", {}) + return self + + @property + def overrides(self): + """Add new rule into the `overrides` RateLimitPolicy section""" + self.spec_section = self.model.spec.setdefault("overrides", {}) + return self def wait_for_ready(self): """Wait for RLP to be enforced""" diff --git a/testsuite/tests/singlecluster/conftest.py b/testsuite/tests/singlecluster/conftest.py index b3656d43..06730f5d 100644 --- a/testsuite/tests/singlecluster/conftest.py +++ b/testsuite/tests/singlecluster/conftest.py @@ -34,10 +34,12 @@ def authorization_name(blame): @pytest.fixture(scope="module") -def authorization(kuadrant, route, authorization_name, cluster, label): +def authorization(request, kuadrant, route, gateway, blame, cluster, label): # pylint: disable=unused-argument """Authorization object (In case of Kuadrant AuthPolicy)""" + target_ref = request.getfixturevalue(getattr(request, "param", "route")) + if kuadrant: - return AuthPolicy.create_instance(cluster, authorization_name, route, labels={"testRun": label}) + return AuthPolicy.create_instance(cluster, blame("authz"), target_ref, labels={"testRun": label}) return None From fd37511f04ed15947c83ce46d4461f5bb45a8b9e Mon Sep 17 00:00:00 2001 From: averevki Date: Tue, 30 Jul 2024 16:59:20 +0200 Subject: [PATCH 2/2] Add tests for defaults Signed-off-by: averevki --- .../tests/singlecluster/defaults/__init__.py | 0 .../defaults/test_basic_authorization.py | 36 +++++++++++++++++++ .../defaults/test_basic_rate_limit.py | 33 +++++++++++++++++ .../defaults/test_rules_exclusivity.py | 35 ++++++++++++++++++ 4 files changed, 104 insertions(+) create mode 100644 testsuite/tests/singlecluster/defaults/__init__.py create mode 100644 testsuite/tests/singlecluster/defaults/test_basic_authorization.py create mode 100644 testsuite/tests/singlecluster/defaults/test_basic_rate_limit.py create mode 100644 testsuite/tests/singlecluster/defaults/test_rules_exclusivity.py diff --git a/testsuite/tests/singlecluster/defaults/__init__.py b/testsuite/tests/singlecluster/defaults/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testsuite/tests/singlecluster/defaults/test_basic_authorization.py b/testsuite/tests/singlecluster/defaults/test_basic_authorization.py new file mode 100644 index 00000000..af535d2d --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/test_basic_authorization.py @@ -0,0 +1,36 @@ +"""Test basic enforcement of the rules inside the 'defaults' block of the AuthPolicy""" + +import pytest + +from testsuite.httpx.auth import HttpxOidcClientAuth + +pytestmark = [pytest.mark.kuadrant_only] + + +@pytest.fixture(scope="module") +def authorization(authorization, oidc_provider): + """Add oidc identity to defaults block of AuthPolicy""" + 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(): + """No RateLimitPolicy is required for this test""" + return None + + +@pytest.mark.parametrize("authorization", ["route", "gateway"], indirect=True) +def test_basic_authorization(authorization, route, client, auth): + """Test that default identity is applied successfully and shows affected status in the route""" + route.refresh() + assert route.is_affected_by(authorization) + + assert client.get("/get").status_code == 401 + assert client.get("/get", auth=auth).status_code == 200 # assert that AuthPolicy is enforced diff --git a/testsuite/tests/singlecluster/defaults/test_basic_rate_limit.py b/testsuite/tests/singlecluster/defaults/test_basic_rate_limit.py new file mode 100644 index 00000000..713bb5da --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/test_basic_rate_limit.py @@ -0,0 +1,33 @@ +"""Test basic enforcement of the rules inside the 'defaults' block of the RateLimitPolicy""" + +import pytest + +from testsuite.kuadrant.policy.rate_limit import Limit + +pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] + +LIMIT = Limit(3, 5) + + +@pytest.fixture(scope="module") +def authorization(): + """No authorization is required for this test""" + return None + + +@pytest.fixture(scope="module") +def rate_limit(rate_limit): + """Add basic requests limit to defaults block of RateLimitPolicy""" + rate_limit.defaults.add_limit("basic", [LIMIT]) + return rate_limit + + +@pytest.mark.parametrize("rate_limit", ["route", "gateway"], indirect=True) +def test_basic_rate_limit(rate_limit, route, client): + """Test that default rate limit is applied successfully and shows affected status in the route""" + route.refresh() + assert route.is_affected_by(rate_limit) + + responses = client.get_many("/get", LIMIT.limit) + responses.assert_all(status_code=200) + assert client.get("/get").status_code == 429 # assert that RateLimitPolicy is enforced diff --git a/testsuite/tests/singlecluster/defaults/test_rules_exclusivity.py b/testsuite/tests/singlecluster/defaults/test_rules_exclusivity.py new file mode 100644 index 00000000..2f8f35bd --- /dev/null +++ b/testsuite/tests/singlecluster/defaults/test_rules_exclusivity.py @@ -0,0 +1,35 @@ +"""Test mutual exclusivity of defaults block and implicit defaults""" + +import pytest +from openshift_client import OpenShiftPythonException + +from testsuite.kuadrant.policy.authorization.auth_policy import AuthPolicy +from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit + +pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] + + +@pytest.fixture(scope="module") +def commit(): + """We need to try to commit objects during the actual test""" + return None + + +def test_rules_exclusivity_authorization(cluster, route, oidc_provider, module_label, blame): + """Test that server will reject object with implicit and explicit defaults defined simultaneously in AuthPolicy""" + authorization = AuthPolicy.create_instance(cluster, blame("authz"), route, labels={"testRun": module_label}) + authorization.defaults.identity.add_oidc("inside-defaults", oidc_provider.well_known["issuer"]) + authorization.identity.add_oidc("outside-defaults", oidc_provider.well_known["issuer"]) + + with pytest.raises(OpenShiftPythonException, match="Implicit and explicit defaults are mutually exclusive"): + authorization.commit() + + +def test_rules_exclusivity_rate_limit(cluster, route, module_label, blame): + """Test that server will reject object with implicit and explicit defaults simultaneously in RateLimitPolicy""" + rate_limit = RateLimitPolicy.create_instance(cluster, blame("limit"), route, labels={"testRun": module_label}) + rate_limit.defaults.add_limit("inside-defaults", [Limit(2, 5)]) + rate_limit.add_limit("outside-defaults", [Limit(2, 5)]) + + with pytest.raises(OpenShiftPythonException, match="Implicit and explicit defaults are mutually exclusive"): + rate_limit.commit()