Skip to content
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

Merged
merged 2 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file.
18 changes: 18 additions & 0 deletions testsuite/tests/singlecluster/identical_hostnames/auth/conftest.py
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():
Copy link
Contributor

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 just authorization

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsmolar force-pushed now

"""
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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixture should be in auth conftest. The same applies for commit in test_identical_hostnames_auth_on_routes.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
It did not occur to me how to handle this difference nicely in the fixture so that it can be used for both tests, that is the reason why I placed it in the test file rather than conftest. Suggestions welcome!

"""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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be in auth conftest

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Empty file.
12 changes: 12 additions & 0 deletions testsuite/tests/singlecluster/identical_hostnames/rlp/conftest.py
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)
Loading
Loading