-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update 'identical hostname' tests since the behavior has changed #599
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
"""Conftest for "identical hostname" tests""" | ||
|
||
import pytest | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def authorization(authorization): | ||
"""1st 'allow-all' Authorization object""" | ||
authorization.authorization.add_opa_policy("rego", "allow = true") | ||
return authorization | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def rate_limit(): | ||
""" | ||
For these tests RateLimitPolicy is not required | ||
""" | ||
return None |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
""" | ||
There used to be a limitation if using one HTTPRoute declaring the same hostname as parent Gateway related to AuthPolicy | ||
https://github.com/Kuadrant/kuadrant-operator/blob/c8d083808daff46772254e223407c849b55020a7/doc/auth.md#limitation-multiple-network-resources-with-identical-hostnames | ||
(second topology mentioned there) | ||
This test validates that it has been properly fixed, i.e. both AuthPolicies are fully and successfully enforced. | ||
""" | ||
|
||
import pytest | ||
|
||
from testsuite.kuadrant.policy import has_condition | ||
from testsuite.kuadrant.policy.authorization.auth_policy import AuthPolicy | ||
|
||
pytestmark = [pytest.mark.kuadrant_only] | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def authorization2(gateway, blame, cluster, label): | ||
"""2nd 'deny-all' Authorization object""" | ||
auth_policy = AuthPolicy.create_instance(cluster, blame("authz2"), gateway, labels={"testRun": label}) | ||
auth_policy.authorization.add_opa_policy("rego", "allow = false") | ||
return auth_policy | ||
|
||
|
||
@pytest.fixture(scope="module", autouse=True) | ||
def commit(request, authorization, authorization2): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixture should be in auth conftest. The same applies for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixture is slightly different for each test. In 'on route' scenario both auth policies are fully enforced whereas in 'on gw' scenario the 2nd authpolicy (the one attached to GW rather than HTTPRoute) is overridden. |
||
"""Ensure Authorizations are created""" | ||
for auth in [authorization, authorization2]: | ||
if auth is not None: | ||
request.addfinalizer(auth.delete) | ||
auth.commit() | ||
auth.wait_for_accepted() | ||
|
||
authorization.wait_for_ready() | ||
|
||
# At this point the 'route2' has not been created yet so authorization2 is completely overridden by authorization1 | ||
assert authorization2.wait_until(has_condition("Enforced", "False", "Overridden")), ( | ||
f"'deny-all' AuthPolicy did not reach expected record status, instead it was: " | ||
f"{authorization2.model.status.condition}" | ||
) | ||
|
||
|
||
def test_identical_hostnames_auth_on_gw_and_route(client, authorization, authorization2): | ||
""" | ||
Tests that Gateway-attached AuthPolicy affects on 'route2' even if both 'route' and 'route2' declare identical | ||
hostname and there is another AuthPolicy already successfully enforced on 'route'. | ||
Setup: | ||
- Two HTTPRoutes declaring identical hostnames but different paths: '/anything/route1' and '/anything/route2' | ||
- 'allow-all' AuthPolicy enforced on the '/anything/route1' HTTPRoute | ||
- 'deny-all' AuthPolicy (created after 'allow-all' AuthPolicy) enforced on the Gateway | ||
Test: | ||
- Send a request via 'route' and assert that response status code is 200 OK | ||
- Send a request via 'route2' and assert that response status code is 403 Forbidden | ||
- Delete the 'allow-all' AuthPolicy | ||
- Send a request via both routes | ||
- Assert that both response status codes are 403 (Forbidden) | ||
""" | ||
|
||
# Verify that the GW-level 'deny-all' AuthPolicy is now only partially enforced ('route2' only). It is overridden | ||
# for 'route1' by HTTPRoute-level 'allow-all' AuthPolicy | ||
authorization2.wait_for_partial_enforced() | ||
|
||
# Access via 'route' is allowed due to 'allow-all' AuthPolicy | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 200 | ||
|
||
# 'deny-all' Gateway AuthPolicy affects route2 | ||
response = client.get("/anything/route2/get") | ||
assert response.status_code == 403 | ||
|
||
# Deletion of 'allow-all' AuthPolicy should make the 'deny-all' Gateway AuthPolicy enforced on both routes | ||
authorization.delete() | ||
authorization2.wait_for_ready() | ||
|
||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 403 | ||
|
||
response = client.get("/anything/route2/get") | ||
assert response.status_code == 403 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
""" | ||
There used to be a limitation if using two HTTPRoutes declaring the same hostname related to AuthPolicy: | ||
https://github.com/Kuadrant/kuadrant-operator/blob/c8d083808daff46772254e223407c849b55020a7/doc/auth.md#limitation-multiple-network-resources-with-identical-hostnames | ||
(the first topology mentioned there) | ||
This test validates that it has been properly fixed, i.e. both AuthPolicies are fully and successfully enforced. | ||
""" | ||
|
||
import pytest | ||
|
||
from testsuite.kuadrant.policy.authorization.auth_policy import AuthPolicy | ||
|
||
pytestmark = [pytest.mark.kuadrant_only] | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def authorization2(route2, blame, cluster, label): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also be in auth conftest There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. For one test this auth policy is attached to HTTPRoute, for the other it is attached to GW. Again, not sure how to handle this nicely so that it can be shared in both tests. |
||
"""2nd 'deny-all' Authorization object""" | ||
auth_policy = AuthPolicy.create_instance(cluster, blame("authz2"), route2, labels={"testRun": label}) | ||
auth_policy.authorization.add_opa_policy("rego", "allow = false") | ||
return auth_policy | ||
|
||
|
||
@pytest.fixture(scope="module", autouse=True) | ||
def commit(request, authorization, authorization2): | ||
"""Ensure Authorizations are created""" | ||
for auth in [authorization, authorization2]: | ||
if auth is not None: | ||
request.addfinalizer(auth.delete) | ||
auth.commit() | ||
auth.wait_for_ready() | ||
|
||
|
||
def test_identical_hostnames_auth_on_routes(client, authorization): | ||
""" | ||
Validate that 2nd AuthPolicy is fully enforced on 'route2' declaring identical hostname as 'route' when another | ||
AuthPolicy already successfully enforced on 'route'. | ||
Setup: | ||
- Two HTTPRoutes declaring identical hostnames but different paths: '/anything/route1' and '/anything/route2' | ||
- 'allow-all' AuthPolicy enforced on the '/anything/route1' HTTPRoute | ||
- 'deny-all' AuthPolicy (created after 'allow-all' AuthPolicy) enforced on the '/anything/route2' HTTPRoute | ||
Test: | ||
- Send a request via 'route' and assert that response status code is 200 OK | ||
- Send a request via 'route2' and assert that response status code is 403 Forbidden | ||
- Delete the 'allow-all' AuthPolicy | ||
- Send a request via both routes | ||
- Assert that access via 'route' is still 200 (OK), deletion of 'allow-all' Authpolicy should have no effect | ||
- Assert that access via 'route2' is still 403 (Forbidden) | ||
""" | ||
|
||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 200 | ||
|
||
response = client.get("/anything/route2/get") | ||
assert response.status_code == 403 | ||
|
||
# Deletion of 'allow-all' AuthPolicy | ||
authorization.delete() | ||
|
||
# Access via 'route' is still allowed because 'deny-all' AuthPolicy is not enforced on this route | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 200 | ||
|
||
# Access via 'route2' is still not allowed due to 'deny-all' AuthPolicy being enforced on 'route2' | ||
response = client.get("/anything/route2/get") | ||
assert response.status_code == 403 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
"""Conftest for "identical hostname" tests""" | ||
|
||
import pytest | ||
|
||
from testsuite.kuadrant.policy.rate_limit import Limit | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def rate_limit(rate_limit): | ||
"""Add limit to 1st RateLimitPolicy allowing 1 request per 10 seconds (a.k.a. '1rp10s' RateLimitPolicy)""" | ||
rate_limit.add_limit("1rp10s", [Limit(1, "10s")]) | ||
return rate_limit |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
""" | ||
There used to be a limitation if using one HTTPRoute declaring the same hostname as parent Gateway related to RLP. | ||
https://github.com/Kuadrant/kuadrant-operator/blob/3b8e313d552090c52d8aadca95f6952f42a03192/doc/rate-limiting.md#limitation-multiple-network-resources-with-identical-hostnames | ||
(second topology mentioned there) | ||
This test validates that it has been properly fixed, i.e. both RateLimitPolicies (RLPs) are successfully enforced. | ||
""" | ||
|
||
from time import sleep | ||
import pytest | ||
|
||
from testsuite.kuadrant.policy import has_condition | ||
from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit | ||
|
||
pytestmark = [pytest.mark.kuadrant_only] | ||
|
||
LIMIT = Limit(2, "10s") | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def rate_limit2(gateway, blame, cluster, label): | ||
"""2nd RateLimitPolicy object allowing 2 requests per 10 seconds (a.k.a. '2rp10s')""" | ||
rlp = RateLimitPolicy.create_instance(cluster, blame("2rp10s"), gateway, labels={"testRun": label}) | ||
rlp.add_limit("2rp10s", [LIMIT]) | ||
return rlp | ||
|
||
|
||
@pytest.fixture(scope="module", autouse=True) | ||
def commit(request, rate_limit, rate_limit2): | ||
"""Ensure RLPs are created""" | ||
for rlp in [rate_limit, rate_limit2]: | ||
if rlp is not None: | ||
request.addfinalizer(rlp.delete) | ||
rlp.commit() | ||
rlp.wait_for_accepted() | ||
|
||
rate_limit.wait_for_ready() | ||
|
||
# At this point the 'route2' has not been created yet so rate_limit2 is completely overridden by rate_limit | ||
assert rate_limit2.wait_until( | ||
has_condition("Enforced", "False", "Overridden") | ||
), f"'2pr10s' RLP did not reach expected record status, instead it was: {rate_limit2.model.status.condition}" | ||
|
||
|
||
def test_identical_hostnames_rlp_on_gw_and_route(client, rate_limit, rate_limit2): | ||
""" | ||
Tests that Gateway-attached RateLimitPolicy is enforced on 'route2' if both 'route' and 'route2' declare | ||
identical hostname and there is another RateLimitPolicy already successfully enforced on 'route'. | ||
Setup: | ||
- Two HTTPRoutes declaring identical hostnames but different paths: '/anything/route1' and '/anything/route2' | ||
- '1rp10s' RateLimitPolicy enforced on the '/anything/route1' HTTPRoute | ||
- '2rp10s' RateLimitPolicy (created after '1pr10s' RateLimitPolicy) enforced on the Gateway | ||
Test: | ||
- Send requests via 'route' and assert that 429 is returned after one 200 OK | ||
- Send a request via 'route2' and assert that 429 is returned after two 200s | ||
- Delete the '1rp10s' RateLimitPolicy | ||
- Send a request via both routes | ||
- Assert that on both routes the 429s are returned after two 200s | ||
""" | ||
# At this point route2 exists so the '2rp10s' RLP should not be overridden, should be partially enforced instead | ||
rate_limit2.wait_for_partial_enforced() | ||
|
||
# Access via 'route' is limited due to '1rp10s' RateLimitPolicy | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 200 | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 429 | ||
|
||
# Access via 'route2' is limited due to '2rp10s' Gateway RateLimitPolicy | ||
responses = client.get_many("/anything/route2/get", LIMIT.limit) | ||
responses.assert_all(status_code=200) | ||
response = client.get("/anything/route2/get") | ||
assert response.status_code == 429 | ||
|
||
# Deletion of '1rp10s' RateLimitPolicy should make both routes rate-limited by '2pr10s' RLP. | ||
# '2pr10s' RLP should get fully enforced (was: partially enforced) | ||
rate_limit.delete() | ||
rate_limit2.wait_for_ready() | ||
|
||
# Wait for 15 seconds to make sure counter is reset | ||
sleep(15) | ||
|
||
responses = client.get_many("/anything/route1/get", LIMIT.limit) | ||
responses.assert_all(status_code=200) | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 429 | ||
|
||
responses = client.get_many("/anything/route2/get", LIMIT.limit) | ||
responses.assert_all(status_code=200) | ||
response = client.get("/anything/route2/get") | ||
assert response.status_code == 429 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
""" | ||
There used to be a limitation if using one HTTPRoute declaring the same hostname as parent Gateway related to RLP. | ||
https://github.com/Kuadrant/kuadrant-operator/blob/3b8e313d552090c52d8aadca95f6952f42a03192/doc/rate-limiting.md#limitation-multiple-network-resources-with-identical-hostnames | ||
(the first topology mentioned there) | ||
This test validates that it has been properly fixed, i.e. both RateLimitPolicies (RLPs) are successfully enforced. | ||
""" | ||
|
||
from time import sleep | ||
import pytest | ||
|
||
from testsuite.kuadrant.policy.rate_limit import RateLimitPolicy, Limit | ||
|
||
|
||
pytestmark = [pytest.mark.kuadrant_only] | ||
|
||
LIMIT = Limit(2, "10s") | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def rate_limit2(route2, blame, cluster, label): | ||
"""2nd RateLimitPolicy allowing 2 requests per 10 seconds (a.k.a. '2rp10s' RateLimitPolicy)""" | ||
rlp = RateLimitPolicy.create_instance(cluster, blame("2rp10s"), route2, labels={"testRun": label}) | ||
rlp.add_limit("2rp10m", [LIMIT]) | ||
return rlp | ||
|
||
|
||
@pytest.fixture(scope="module", autouse=True) | ||
def commit(request, rate_limit, rate_limit2): | ||
"""Ensure Authorizations are created""" | ||
for rlp in [rate_limit, rate_limit2]: | ||
if rlp is not None: | ||
request.addfinalizer(rlp.delete) | ||
rlp.commit() | ||
rlp.wait_for_ready() | ||
|
||
|
||
def test_identical_hostnames_rlp_on_routes(client, rate_limit2): | ||
""" | ||
Validates that 1st RateLimitPolicy is still enforced on 'route' declaring identical hostname as 'route2' if another | ||
RateLimitPolicy got successfully enforced on 'route2' in the interim. | ||
Setup: | ||
- Two HTTPRoutes declaring identical hostnames but different paths: '/anything/route1' and '/anything/route2' | ||
- '1rp10m' RateLimitPolicy enforced on the '/anything/route1' HTTPRoute | ||
- '2rp10m' RateLimitPolicy (created after '1rp10s' RateLimitPolicy) enforced on the '/anything/route2' HTTPRoute | ||
Test: | ||
- Send a request via 'route' and assert that no 429s (Too Many Requests) are returned | ||
- Send a request via 'route2' and assert that no 429s (Too Many Requests) are returned | ||
- Delete the '2rp10s' RateLimitPolicy | ||
- Send a request via both routes | ||
- Assert that 429 is returned after single 200 (OK) for route1 | ||
- Assert that there are no 429s for 'route2' | ||
""" | ||
# Access via 'route' is limited due to '1rp10s' RateLimitPolicy | ||
# despite it reporting being successfully enforced | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 200 | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 429 | ||
|
||
# Access via 'route2' is limited due to '2rp10s' RateLimitPolicy | ||
responses = client.get_many("/anything/route2/get", LIMIT.limit) | ||
responses.assert_all(status_code=200) | ||
response = client.get("/anything/route2/get") | ||
assert response.status_code == 429 | ||
|
||
# Deletion of '2rp10m' RateLimitPolicy | ||
rate_limit2.delete() | ||
|
||
# Access via 'route' should now be still limited via '1rp10s' RateLimitPolicy | ||
# Wait for 15s to make sure the counter is reset | ||
sleep(15) | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 200 | ||
response = client.get("/anything/route1/get") | ||
assert response.status_code == 429 | ||
|
||
# Access via 'route2' is now not limited at all | ||
responses = client.get_many("/anything/route2/get", LIMIT.limit + 1) | ||
responses.assert_all(status_code=200) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Common practice across testsuite is creating new
commit
fixtures, with objects you want to create. In this case justauthorization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in theory this can be removed I assume. I have a
commit
fixture defined in the test files directly and RLP is not committed there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems it indeed can be removed. Will force push and let you know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsmolar force-pushed now