diff --git a/testsuite/gateway/gateway_api/gateway.py b/testsuite/gateway/gateway_api/gateway.py index a9963a53..aa03759a 100644 --- a/testsuite/gateway/gateway_api/gateway.py +++ b/testsuite/gateway/gateway_api/gateway.py @@ -78,7 +78,8 @@ def is_affected_by(self, policy: Policy) -> bool: f"kuadrant.io/{policy.kind(lowercase=False)}Affected", "True", "Accepted", - f"Object affected by {policy.kind(lowercase=False)} [{policy.namespace()}/{policy.name()}]", + f"Object affected by {policy.kind(lowercase=False)}", + f"{policy.namespace()}/{policy.name()}", ): return True return False diff --git a/testsuite/gateway/gateway_api/route.py b/testsuite/gateway/gateway_api/route.py index 50a9cd7a..10f01cc9 100644 --- a/testsuite/gateway/gateway_api/route.py +++ b/testsuite/gateway/gateway_api/route.py @@ -54,7 +54,8 @@ def is_affected_by(self, policy: Policy): f"kuadrant.io/{policy.kind(lowercase=False)}Affected", "True", "Accepted", - f"Object affected by {policy.kind(lowercase=False)} [{policy.namespace()}/{policy.name()}]", + f"Object affected by {policy.kind(lowercase=False)}", + f"{policy.namespace()}/{policy.name()}", ): return True return False diff --git a/testsuite/kuadrant/policy/__init__.py b/testsuite/kuadrant/policy/__init__.py index 8b0c6ccf..d4ba2ad8 100644 --- a/testsuite/kuadrant/policy/__init__.py +++ b/testsuite/kuadrant/policy/__init__.py @@ -42,7 +42,8 @@ def _check(obj): f"kuadrant.io/{policy.kind(lowercase=False)}Affected", "True", "Accepted", - f"Object affected by {policy.kind(lowercase=False)} {policy.namespace()}/{policy.name()}", + f"Object affected by {policy.kind(lowercase=False)}", + f"{policy.namespace()}/{policy.name()}", ): return True return False diff --git a/testsuite/tests/singlecluster/overrides/test_basic_auth.py b/testsuite/tests/singlecluster/overrides/test_basic_auth.py index d00e083d..34d2b295 100644 --- a/testsuite/tests/singlecluster/overrides/test_basic_auth.py +++ b/testsuite/tests/singlecluster/overrides/test_basic_auth.py @@ -3,17 +3,15 @@ import pytest from testsuite.httpx.auth import HttpxOidcClientAuth -from testsuite.kuadrant.policy.authorization.auth_policy import AuthPolicy pytestmark = [pytest.mark.kuadrant_only] @pytest.fixture(scope="module") -def authorization(route, gateway, blame, cluster, label, oidc_provider): # pylint: disable=unused-argument - """Add oidc identity to overrides block of gateway-attached AuthPolicy""" - auth_policy = AuthPolicy.create_instance(cluster, blame("authz"), gateway, labels={"testRun": label}) - auth_policy.overrides.identity.add_oidc("override", oidc_provider.well_known["issuer"]) - return auth_policy +def authorization(authorization, oidc_provider): + """Add oidc identity to defaults block of AuthPolicy""" + authorization.overrides.identity.add_oidc("override", oidc_provider.well_known["issuer"]) + return authorization @pytest.fixture(scope="module") @@ -28,12 +26,12 @@ def rate_limit(): return None +@pytest.mark.parametrize("authorization", ["route", "gateway"], indirect=True) def test_basic_auth(route, authorization, client, auth): """Test if rules inside overrides block of Gateway's AuthPolicy are inherited by the HTTPRoute and enforced like any other normal rule""" route.refresh() assert route.is_affected_by(authorization) - response = client.get("/get") - assert response.status_code == 401 + 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/overrides/test_basic_rate_limit.py b/testsuite/tests/singlecluster/overrides/test_basic_rate_limit.py index 9b0460ba..dbd3fae6 100644 --- a/testsuite/tests/singlecluster/overrides/test_basic_rate_limit.py +++ b/testsuite/tests/singlecluster/overrides/test_basic_rate_limit.py @@ -1,4 +1,4 @@ -"""Test basic enforcement of the rules inside the 'overrides' block of the RateLimitPolicy assigned to a Gateway""" +"""Test enforcement of the rules inside the 'overrides' block of the RateLimitPolicy assigned to a Gateway/HTTPRoute""" import pytest @@ -6,7 +6,7 @@ pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] -GATEWAY_LIMIT = Limit(3, "5s") +OVERRIDE_LIMIT = Limit(3, "5s") ROUTE_LIMIT = Limit(2, "5s") @@ -16,33 +16,34 @@ def authorization(): return None -@pytest.fixture(scope="module") -def rate_limit_gw(request, cluster, blame, module_label, gateway): - """Add a RateLimitPolicy to the Gateway with an overrides block to override the Route-level policy.""" - rate_limit_gateway = RateLimitPolicy.create_instance( - cluster, blame("limit-gateway"), gateway, labels={"testRun": module_label} +@pytest.fixture(scope="function") +def rate_limit_route(request, cluster, blame, module_label, route): + """Add a RateLimitPolicy to the HTTPRoute with a basic limit to be overriden.""" + rate_limit_route = RateLimitPolicy.create_instance( + cluster, blame("limit-route"), route, labels={"testRun": module_label} ) - rate_limit_gateway.overrides.add_limit("basic", [GATEWAY_LIMIT]) - request.addfinalizer(rate_limit_gateway.delete) - rate_limit_gateway.commit() - rate_limit_gateway.wait_for_ready() - return rate_limit_gateway + rate_limit_route.add_limit("basic", [ROUTE_LIMIT]) + request.addfinalizer(rate_limit_route.delete) + rate_limit_route.commit() + rate_limit_route.wait_for_accepted() + return rate_limit_route @pytest.fixture(scope="module") def rate_limit(rate_limit): - """Add basic requests limit to RateLimitPolicy""" - rate_limit.add_limit("basic", [ROUTE_LIMIT]) + """Add an override to RateLimitPolicy""" + rate_limit.overrides.add_limit("override-limit", [OVERRIDE_LIMIT]) return rate_limit -def test_basic_rate_limit(rate_limit, rate_limit_gw, route, client): - """Test if rules inside overrides block of Gateway's RateLimitPolicy are inherited by the HTTPRoute - and enforced like any other normal rule""" +@pytest.mark.parametrize("rate_limit", ["route", "gateway"], indirect=True) +def test_basic_rate_limit(rate_limit, rate_limit_route, route, client): + """Test if rules inside overrides block of Gateway/HTTPRoute RateLimitPolicy are inherited by the HTTPRoute + and override the rate limit targeting the route.""" route.refresh() assert route.is_affected_by(rate_limit) - rate_limit_gw.wait_for_full_enforced() + assert route.is_affected_by(rate_limit_route) - responses = client.get_many("/get", GATEWAY_LIMIT.limit) + responses = client.get_many("/get", OVERRIDE_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/overrides/test_route_override.py b/testsuite/tests/singlecluster/overrides/test_route_override.py deleted file mode 100644 index 64c161de..00000000 --- a/testsuite/tests/singlecluster/overrides/test_route_override.py +++ /dev/null @@ -1,43 +0,0 @@ -"""Test that overrides block can not be defined in AuthPolicy and RateLimitPolicy attached to a HTTPRoute""" - -import pytest -from openshift_client import OpenShiftPythonException - -from testsuite.kuadrant.policy.rate_limit import Limit - -pytestmark = [pytest.mark.kuadrant_only, pytest.mark.limitador] - - -@pytest.fixture(scope="module") -def authorization(authorization, oidc_provider): - """Create AuthPolicy with basic oidc rules in the overrides block""" - authorization.overrides.identity.add_oidc("override", oidc_provider.well_known["issuer"]) - return authorization - - -@pytest.fixture(scope="module") -def rate_limit(rate_limit): - """Add basic rate limiting rules in the overrides block""" - rate_limit.overrides.add_limit("override", [Limit(2, "5s")]) - return rate_limit - - -@pytest.fixture(scope="module") -def commit(): - """We need to try to commit objects during the actual test""" - return None - - -@pytest.mark.parametrize( - "component_fixture", - [ - pytest.param("authorization", id="AuthPolicy"), - pytest.param("rate_limit", id="RateLimitPolicy"), - ], -) -@pytest.mark.issue("https://github.com/Kuadrant/kuadrant-operator/issues/775") -def test_route_override(request, component_fixture): - """Test that server will reject policy attached to a HTTPRoute containing an overrides block""" - component = request.getfixturevalue(component_fixture) - with pytest.raises(OpenShiftPythonException, match="Overrides are.*"): - component.commit() diff --git a/testsuite/utils.py b/testsuite/utils.py index c5fbf388..069ade02 100644 --- a/testsuite/utils.py +++ b/testsuite/utils.py @@ -168,12 +168,13 @@ def _asdict_recurse(obj): return result -def check_condition(condition, condition_type, status, reason=None, message=None): +def check_condition(condition, condition_type, status, reason=None, message=None, policy=None): """Checks if condition matches expectation, won't check message and reason if they are None""" if ( # pylint: disable=too-many-boolean-expressions condition.type == condition_type and condition.status == status and (message is None or message in condition.message) + and (policy is None or policy in condition.message) and (reason is None or reason == condition.reason) ): return True