Add tests for mTLS configuration and functional behavior #675

emmaaroche · 2025-05-06T11:09:55Z

Description:

This PR introduces tests that validate Kuadrant's mTLS functionality.

The tests are grouped into configuration and functional behavior checks, and are applied across three scenarios:

AuthPolicy only
RateLimitPolicy only
Both policies applied together

Each scenario includes the following tests:

mTLS Functional Behavior Tests:
- Ensures that requests succeed when mTLS is disabled
- Verifies that requests continue to succeed when mTLS is enabled
- Confirms that requests still succeed after mTLS is disabled again
mTLS Configuration Tests:
- Verifies that PeerAuthentication with STRICT mode is created when mTLS is enabled
- Confirms that the relevant pods have the necessary Istio sidecar and labels after mTLS is enabled
- Ensures Kuadrant CR reaches Ready status after enabling mTLS

Related Issues / PR's:

#1230
#1329
#1170

azgabur

WIP Review

testsuite/tests/singlecluster/mtls/conftest.py

testsuite/tests/singlecluster/mtls/test_mtls_configuration.py

testsuite/tests/singlecluster/mtls/conftest.py

testsuite/tests/singlecluster/mtls/test_mtls_configuration.py

azgabur

test_mtls_configuration_with_* could be put in one file with some parametrization
same with test_mtls_functional_behavior_*

https://docs.pytest.org/en/7.1.x/example/parametrize.html

Or look how its done for example here:
testsuite/tests/kuadrantctl/cli/test_simple_route.py
or
testsuite/tests/singlecluster/authorino/operator/clusterwide/test_clusterwide.py

testsuite/tests/singlecluster/mtls/conftest.py

testsuite/tests/singlecluster/mtls/test_mtls_configuration_with_authpolicy.py

testsuite/tests/singlecluster/mtls/test_mtls_functional_behavior_with_authpolicy.py

azgabur

Good job with the parametrization!

azgabur · 2025-05-30T10:36:15Z

testsuite/tests/singlecluster/gateway/mtls/test_mtls_behaviour.py

+
+pytestmark = [pytest.mark.kuadrant_only, pytest.mark.disruptive]
+
+component_cases = ["limitador", "authorino", "both"]


What if instead of defining a new symbol "both" you would use lists like this:

Suggested change

component_cases = ["limitador", "authorino", "both"]

component_cases = [["limitador"], ["authorino"], ["limitador", "authorino"]]

That could simplify later logic.

azgabur · 2025-05-30T10:41:50Z

testsuite/tests/singlecluster/gateway/mtls/test_mtls_behaviour.py

+    if component == "limitador":
+        rate_limit.wait_for_ready()
+
+        responses = client.get_many("/get", 2)
+        responses.assert_all(status_code=200)
+        assert client.get("/get").status_code == 429
+
+    elif component == "authorino":
+        authorization.wait_for_ready()
+        response = client.get("/get", auth=auth)
+
+        assert response.status_code == 200
+
+        response = client.get("/get")
+        assert response.status_code == 401
+
+    elif component == "both":
+        authorization.wait_for_ready()
+        rate_limit.wait_for_ready()
+
+        response = client.get("/get", auth=auth)
+        assert response.status_code == 200
+
+        response = client.get("/get")
+        assert response.status_code == 401
+
+        response = client.get("/get", auth=auth)
+        assert response.status_code == 429


For example this could change to

if limitador in component: ... if authorino in component: ...

To prevent authorino testing with "auth" interfering with limitador testing with "get_many" you can create "when" rule for different paths. For example have path /anything/limitador being ratelimited and /anything/authorino requiring authentication.

I implemented all the changes you suggested in relation to this, but I have still had to add "auth" to the limitador requests when authorino is also enabled because even with a when condition, the AuthPolicy was still causing 401s on /anything/limitador.

Hmm, maybe its due to the rule that if at least one identity is defined than at least one must match otherwise the default response is 401. Maybe adding anonymous auth for the /anything/limitador could solve this 🤔

Will give this a try!

That has worked, thanks 😄

azgabur · 2025-05-30T10:43:07Z

testsuite/tests/singlecluster/gateway/mtls/conftest.py

+def rate_limit(cluster, blame, module_label, gateway, route):  # pylint: disable=unused-argument
+    """RateLimitPolicy for testing"""
+    policy = RateLimitPolicy.create_instance(cluster, blame("limit"), gateway, labels={"testRun": module_label})
+    policy.add_limit("basic", [Limit(2, "10s")])


For example you can add when rule like this.

Suggested change

policy.add_limit("basic", [Limit(2, "10s")])

policy.add_limit("basic", [Limit(2, "10s")], when=[CelPredicate("request.path == '/anything/limitador'")])

azgabur · 2025-05-30T10:46:51Z

testsuite/tests/singlecluster/gateway/mtls/conftest.py

+@pytest.fixture(scope="module")
+def authorization(component, request):
+    """Enable AuthPolicy when component is 'authorino' or 'both'"""
+    if component in ("authorino", "both"):


And for authorization having bit trickier way but still possible:

authorization.identity.clear_all() authorization.identity.add_oidc("default", oidc_provider.well_known["issuer"], when=[CelPredicate("request.path == '/anything/authorino'"))

azgabur · 2025-05-30T10:48:14Z

testsuite/tests/singlecluster/gateway/mtls/conftest.py

+    request.addfinalizer(_reset)
+
+
+def get_components_to_check(component):


Then you would not need this helper function

azgabur · 2025-05-30T10:58:02Z

testsuite/tests/singlecluster/gateway/mtls/test_mtls_behaviour.py

+    """Tests that requests succeed when mTLS is disabled"""
+    configure_mtls(False)
+
+    assert kuadrant.model.spec.mtls.enable is False


is operator is usually used for checking if the variable is None (due to Python considering certain non-bool values as bool ones, like empty [] list and so on) not really for normal equality checking, thats what == is for.
Yes is operator can also be used if you really want to know two variables point to the same object and are not just equal, but not sure if we use such equality checking in our testsuite.

In this case it will still work as these primitive type objects in python have all same id value - which is what is operator uses for comparison.

azgabur · 2025-05-30T11:09:14Z

testsuite/tests/singlecluster/gateway/mtls/test_mtls_behaviour.py

+
+
+@pytest.mark.parametrize("component", component_cases, indirect=True)
+def test_requests_still_succeed_after_mtls_disabled_again(


This test case depends on the test order. Yes normally all test functions run one after other in a module file but this is not guaranteed. What if you run this test on its own? Generally you would want to avoid test cases which depend on other test cases as well. So what I would maybe do is add this to the beginning of the test to ensure the mtls was on before:

configure_mtls(True) ...wait for status here...

Adn than continue with disabling mtls and checking

azgabur · 2025-05-30T11:17:21Z

testsuite/tests/singlecluster/gateway/mtls/test_mtls_configuration.py

+        peer_auths = selector("peerauthentication").objects()
+        assert peer_auths, "No PeerAuthentication resources found"
+        strict = [pa.name() for pa in peer_auths if pa.model.spec.mtls.mode == "STRICT"]
+        assert strict, "No PeerAuthentication with mtls.mode == 'STRICT'"


Please add one more assert checking that:

spec.selector.matchLabels == {kuadrant.io/managed: 'true'}

azgabur · 2025-05-30T11:23:23Z

testsuite/tests/singlecluster/gateway/mtls/test_mtls_configuration.py

+        wait_for_status(kuadrant, expected=True, component=comp)
+
+    for comp in components_to_check:
+        pod = wait_for_injected_pod(comp)


Do a assert if the pod is None which means the pod with sidecar was not created

Signed-off-by: emmaaroche <[email protected]>

emmaaroche self-assigned this May 6, 2025

emmaaroche added the Test case New test case label May 6, 2025

emmaaroche requested a review from azgabur May 6, 2025 11:10

azgabur reviewed May 6, 2025

View reviewed changes

emmaaroche linked an issue May 21, 2025 that may be closed by this pull request

Add e2e tests for mTLS Configuration via Kuadrant CR Kuadrant/kuadrant-operator#1230

Open

emmaaroche force-pushed the mtls-config-e2e-tests branch from 20a477f to c3dbc9d Compare May 21, 2025 12:56

emmaaroche requested a review from azgabur May 21, 2025 13:02

azgabur reviewed May 22, 2025

View reviewed changes

emmaaroche force-pushed the mtls-config-e2e-tests branch from c3dbc9d to fc1902c Compare May 27, 2025 07:43

emmaaroche requested a review from azgabur May 27, 2025 07:45

azgabur reviewed May 30, 2025

View reviewed changes

Add tests for mTLS configuration and functional behavior

98c716e

Signed-off-by: emmaaroche <[email protected]>

emmaaroche force-pushed the mtls-config-e2e-tests branch from fc1902c to 98c716e Compare June 3, 2025 13:07

emmaaroche requested a review from azgabur June 3, 2025 13:09


		pytestmark = [pytest.mark.kuadrant_only, pytest.mark.disruptive]

		component_cases = ["limitador", "authorino", "both"]

	component_cases = ["limitador", "authorino", "both"]
	component_cases = [["limitador"], ["authorino"], ["limitador", "authorino"]]

	policy.add_limit("basic", [Limit(2, "10s")])
	policy.add_limit("basic", [Limit(2, "10s")], when=[CelPredicate("request.path == '/anything/limitador'")])

		request.addfinalizer(_reset)


		def get_components_to_check(component):



		@pytest.mark.parametrize("component", component_cases, indirect=True)
		def test_requests_still_succeed_after_mtls_disabled_again(

Add tests for mTLS configuration and functional behavior #675

Are you sure you want to change the base?

Add tests for mTLS configuration and functional behavior #675

Uh oh!

Conversation

emmaaroche commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Related Issues / PR's:

Uh oh!

azgabur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azgabur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azgabur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emmaaroche commented May 6, 2025 •

edited

Loading