From 108f930d4eec4dc3759add43637c3e41e0133135 Mon Sep 17 00:00:00 2001 From: averevki Date: Wed, 2 Aug 2023 16:48:43 +0200 Subject: [PATCH 1/3] Refactor AuthConfig response section --- testsuite/objects/__init__.py | 12 ++++ .../openshift/objects/auth_config/sections.py | 61 ++++++++++++++++--- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/testsuite/objects/__init__.py b/testsuite/objects/__init__.py index d147edad..f5dbd5ed 100644 --- a/testsuite/objects/__init__.py +++ b/testsuite/objects/__init__.py @@ -97,6 +97,18 @@ def asdict(self): return {"valueFrom": {"authJSON": self.authJSON}} +@dataclass +class Property: + """Dataclass for static and dynamic values. Property is a Value with name.""" + + name: str + value: ABCValue + + def asdict(self): + """Override `asdict` function""" + return {"name": self.name, **asdict(self.value)} + + @dataclass class Cache: """Dataclass for specifying Cache in Authorization""" diff --git a/testsuite/openshift/objects/auth_config/sections.py b/testsuite/openshift/objects/auth_config/sections.py index 0e0217a0..d5a58349 100644 --- a/testsuite/openshift/objects/auth_config/sections.py +++ b/testsuite/openshift/objects/auth_config/sections.py @@ -7,6 +7,8 @@ Rule, Cache, ABCValue, + ValueFrom, + Property, ) from testsuite.openshift.objects import modify @@ -175,20 +177,61 @@ def uma_metadata(self, name, endpoint, credentials, **common_features): class Responses(Section): """Section which contains response configuration""" - def add_simple(self, auth_json, name="simple", key="data", **common_features): - """Adds simple response to AuthConfig""" - self.add( + def _add( + self, + name: str, + value: dict, + wrapper_key: str = None, + wrapper: Literal["httpHeader", "envoyDynamicMetadata"] = None, + **common_features + ): + """Add response to AuthConfig""" + if wrapper: + value["wrapper"] = wrapper + if wrapper_key: + value["wrapperKey"] = wrapper_key + + self.add_item(name, value, **common_features) + + def add_simple(self, auth_json: str, name="simple", key="data", **common_features): + """Add simple response to AuthConfig""" + self.json(name, [Property(key, ValueFrom(auth_json))], **common_features) + + @modify + def json(self, name: str, properties: list[Property], **common_features): + """Adds json response to AuthConfig""" + asdict_properties = [asdict(p) for p in properties] + self._add(name, {"json": {"properties": asdict_properties}}, **common_features) + + @modify + def plain(self, name: str, value: ABCValue, **common_features): + """Adds plain response to AuthConfig""" + self._add(name, {"plain": asdict(value)}, **common_features) + + @modify + def wristband(self, name: str, issuer: str, secret_name: str, algorithm: str = "RS256", **common_features): + """Adds wristband response to AuthConfig""" + self._add( + name, { "name": name, - "json": {"properties": [{"name": key, "valueFrom": {"authJSON": auth_json}}]}, - **common_features, - } + "wristband": { + "issuer": issuer, + "signingKeyRefs": [ + { + "name": secret_name, + "algorithm": algorithm, + } + ], + }, + }, + **common_features ) @modify - def add(self, response, **common_features): - """Adds response section to AuthConfig.""" - self.add_item(response.pop("name"), response, **common_features) + def remove_all(self): + """Removes all responses from AuthConfig""" + self.section.clear() class Authorizations(Section): From 1adb64a3ca386c1322b7a619b8837e5b62764815 Mon Sep 17 00:00:00 2001 From: averevki Date: Wed, 12 Jul 2023 15:18:46 +0200 Subject: [PATCH 2/3] Refactor response tests due to the new section structure --- testsuite/openshift/objects/__init__.py | 4 ++- .../kuadrant/authorino/response/conftest.py | 31 ------------------- .../authorino/response/test_auth_json.py | 11 +++++-- .../authorino/response/test_base64.py | 23 +++++--------- .../response/test_multiple_responses.py | 13 ++++---- .../response/test_simple_response.py | 9 ++++-- .../authorino/response/test_wrapper_key.py | 12 ++++--- 7 files changed, 39 insertions(+), 64 deletions(-) delete mode 100644 testsuite/tests/kuadrant/authorino/response/conftest.py diff --git a/testsuite/openshift/objects/__init__.py b/testsuite/openshift/objects/__init__.py index e2165a42..9314ef82 100644 --- a/testsuite/openshift/objects/__init__.py +++ b/testsuite/openshift/objects/__init__.py @@ -46,4 +46,6 @@ def commit(self): def delete(self, ignore_not_found=True, cmd_args=None): """Deletes the resource, by default ignored not found""" - return super().delete(ignore_not_found, cmd_args) + deleted = super().delete(ignore_not_found, cmd_args) + self.committed = False + return deleted diff --git a/testsuite/tests/kuadrant/authorino/response/conftest.py b/testsuite/tests/kuadrant/authorino/response/conftest.py deleted file mode 100644 index 1a4f9127..00000000 --- a/testsuite/tests/kuadrant/authorino/response/conftest.py +++ /dev/null @@ -1,31 +0,0 @@ -"""Conftest for custom Response tests""" -import pytest - - -@pytest.fixture(scope="module") -def responses(): - """Returns responses to be added to the AuthConfig""" - return [] - - -# pylint: disable=unused-argument -@pytest.fixture(scope="module") -def authorization_name(blame, responses): - """Ensure for every response we have a unique authorization""" - return blame("authz") - - -@pytest.fixture(scope="module") -def authorization(authorization, responses): - """Add response to Authorization""" - for response in responses: - authorization.responses.add(response) - return authorization - - -# pylint: disable=unused-argument -@pytest.fixture(scope="function", autouse=True) -def commit(request, authorization, responses): - """Commits all important stuff before tests""" - request.addfinalizer(authorization.delete) - authorization.commit() diff --git a/testsuite/tests/kuadrant/authorino/response/test_auth_json.py b/testsuite/tests/kuadrant/authorino/response/test_auth_json.py index a820ceb9..a882e7ef 100644 --- a/testsuite/tests/kuadrant/authorino/response/test_auth_json.py +++ b/testsuite/tests/kuadrant/authorino/response/test_auth_json.py @@ -4,6 +4,8 @@ import pytest +from testsuite.objects import Property, ValueFrom + @pytest.fixture(scope="module") def issuer(oidc_provider): @@ -24,10 +26,13 @@ def path_and_value(request): @pytest.fixture(scope="module") -def responses(path_and_value): - """Returns response to be added to the AuthConfig""" +def authorization(authorization, path_and_value): + """Add response to Authorization""" path, _ = path_and_value - return [{"name": "header", "json": {"properties": [{"name": "anything", "valueFrom": {"authJSON": path}}]}}] + + authorization.responses.remove_all() # delete previous responses due to the parametrization + authorization.responses.json("header", [Property("anything", ValueFrom(path))]) + return authorization def test_auth_json_path(auth, client, path_and_value): diff --git a/testsuite/tests/kuadrant/authorino/response/test_base64.py b/testsuite/tests/kuadrant/authorino/response/test_base64.py index a491a643..d5b2b320 100644 --- a/testsuite/tests/kuadrant/authorino/response/test_base64.py +++ b/testsuite/tests/kuadrant/authorino/response/test_base64.py @@ -6,23 +6,16 @@ import pytest +from testsuite.objects import Property, ValueFrom + @pytest.fixture(scope="module") -def responses(): - """Returns response to be added to the AuthConfig""" - return [ - { - "name": "header", - "json": { - "properties": [ - { - "name": "anything", - "valueFrom": {"authJSON": "context.request.http.headers.test|@base64:decode"}, - } - ] - }, - } - ] +def authorization(authorization): + """Add response to Authorization""" + authorization.responses.json( + "header", [Property("anything", ValueFrom("context.request.http.headers.test|@base64:decode"))] + ) + return authorization @pytest.mark.parametrize( diff --git a/testsuite/tests/kuadrant/authorino/response/test_multiple_responses.py b/testsuite/tests/kuadrant/authorino/response/test_multiple_responses.py index 59fdfd74..b6aef431 100644 --- a/testsuite/tests/kuadrant/authorino/response/test_multiple_responses.py +++ b/testsuite/tests/kuadrant/authorino/response/test_multiple_responses.py @@ -3,14 +3,15 @@ import pytest +from testsuite.objects import Property, Value + @pytest.fixture(scope="module") -def responses(): - """Returns response to be added to the AuthConfig""" - return [ - {"name": "Header", "json": {"properties": [{"name": "anything", "value": "one"}]}}, - {"name": "X-Test", "json": {"properties": [{"name": "anything", "value": "two"}]}}, - ] +def authorization(authorization): + """Add response to Authorization""" + authorization.responses.json("header", [Property("anything", Value("one"))]) + authorization.responses.json("X-Test", [Property("anything", Value("two"))]) + return authorization def test_multiple_responses(auth, client): diff --git a/testsuite/tests/kuadrant/authorino/response/test_simple_response.py b/testsuite/tests/kuadrant/authorino/response/test_simple_response.py index ec2fb33f..7fb41696 100644 --- a/testsuite/tests/kuadrant/authorino/response/test_simple_response.py +++ b/testsuite/tests/kuadrant/authorino/response/test_simple_response.py @@ -3,11 +3,14 @@ import pytest +from testsuite.objects import Property, Value + @pytest.fixture(scope="module") -def responses(): - """Returns response to be added to the AuthConfig""" - return [{"name": "header", "json": {"properties": [{"name": "anything", "value": "one"}]}}] +def authorization(authorization): + """Add response to Authorization""" + authorization.responses.json("header", [Property("anything", Value("one"))]) + return authorization def test_simple_response_with(auth, client): diff --git a/testsuite/tests/kuadrant/authorino/response/test_wrapper_key.py b/testsuite/tests/kuadrant/authorino/response/test_wrapper_key.py index 289d5fd4..1335c298 100644 --- a/testsuite/tests/kuadrant/authorino/response/test_wrapper_key.py +++ b/testsuite/tests/kuadrant/authorino/response/test_wrapper_key.py @@ -3,6 +3,8 @@ import pytest +from testsuite.objects import Property, Value + @pytest.fixture(scope="module", params=["123456789", "standardCharacters", "specialcharacters+*-."]) def header_name(request): @@ -11,11 +13,11 @@ def header_name(request): @pytest.fixture(scope="module") -def responses(header_name): - """Returns response to be added to the AuthConfig""" - return [ - {"name": "header", "wrapperKey": header_name, "json": {"properties": [{"name": "anything", "value": "one"}]}} - ] +def authorization(authorization, header_name): + """Add response to Authorization""" + authorization.responses.remove_all() # delete previous responses due to the parametrization + authorization.responses.json("header", [Property("anything", Value("one"))], wrapper_key=header_name) + return authorization def test_wrapper_key_with(auth, client, header_name): From 29fd00f741a90649ebd911971656337754131061 Mon Sep 17 00:00:00 2001 From: averevki Date: Wed, 2 Aug 2023 16:49:43 +0200 Subject: [PATCH 3/3] Refactor other response tests due to the new section structure --- .../test_response_condition.py | 14 +++----------- .../identity/rhsso/test_rhsso_context.py | 18 ++++++++---------- .../clusterwide/test_wildcard_collision.py | 5 +++-- .../authorino/operator/http/conftest.py | 10 ++++------ .../authorino/operator/sharding/conftest.py | 3 ++- .../kuadrant/authorino/wristband/conftest.py | 17 +---------------- 6 files changed, 21 insertions(+), 46 deletions(-) diff --git a/testsuite/tests/kuadrant/authorino/conditions/section_conditions/test_response_condition.py b/testsuite/tests/kuadrant/authorino/conditions/section_conditions/test_response_condition.py index 61f384b1..ee3e9ffe 100644 --- a/testsuite/tests/kuadrant/authorino/conditions/section_conditions/test_response_condition.py +++ b/testsuite/tests/kuadrant/authorino/conditions/section_conditions/test_response_condition.py @@ -1,23 +1,15 @@ """Test condition to skip the response section of AuthConfig""" import pytest -from testsuite.objects import Rule +from testsuite.objects import Property, Rule, Value from testsuite.utils import extract_response @pytest.fixture(scope="module") def authorization(authorization): """Add to the AuthConfig response, which will only trigger on POST requests""" - authorization.responses.add( - { - "name": "simple", - "json": { - "properties": [ - {"name": "data", "value": "response"}, - ] - }, - }, - when=[Rule("context.request.http.method", "eq", "POST")], + authorization.responses.json( + "simple", [Property("data", Value("response"))], when=[Rule("context.request.http.method", "eq", "POST")] ) return authorization diff --git a/testsuite/tests/kuadrant/authorino/identity/rhsso/test_rhsso_context.py b/testsuite/tests/kuadrant/authorino/identity/rhsso/test_rhsso_context.py index f1b2065f..6e16023c 100644 --- a/testsuite/tests/kuadrant/authorino/identity/rhsso/test_rhsso_context.py +++ b/testsuite/tests/kuadrant/authorino/identity/rhsso/test_rhsso_context.py @@ -4,20 +4,18 @@ import pytest +from testsuite.objects import Property, ValueFrom + @pytest.fixture(scope="module") def authorization(authorization): """Setup AuthConfig for test""" - authorization.responses.add( - { - "name": "auth-json", - "json": { - "properties": [ - {"name": "auth", "valueFrom": {"authJSON": "auth.identity"}}, - {"name": "context", "valueFrom": {"authJSON": "context.request.http.headers.authorization"}}, - ] - }, - } + authorization.responses.json( + "auth-json", + [ + Property("auth", ValueFrom("auth.identity")), + Property("context", ValueFrom("context.request.http.headers.authorization")), + ], ) return authorization diff --git a/testsuite/tests/kuadrant/authorino/operator/clusterwide/test_wildcard_collision.py b/testsuite/tests/kuadrant/authorino/operator/clusterwide/test_wildcard_collision.py index db274faa..28b462f7 100644 --- a/testsuite/tests/kuadrant/authorino/operator/clusterwide/test_wildcard_collision.py +++ b/testsuite/tests/kuadrant/authorino/operator/clusterwide/test_wildcard_collision.py @@ -4,6 +4,7 @@ import pytest +from testsuite.objects import Property, Value from testsuite.openshift.objects.auth_config import AuthConfig @@ -14,7 +15,7 @@ def authorization(authorino, blame, openshift, module_label, envoy, wildcard_dom auth = AuthConfig.create_instance( openshift, blame("ac"), None, hostnames=[wildcard_domain], labels={"testRun": module_label} ) - auth.responses.add({"name": "header", "json": {"properties": [{"name": "anything", "value": "one"}]}}) + auth.responses.json("header", [Property("anything", Value("one"))]) return auth @@ -25,7 +26,7 @@ def authorization2(authorino, blame, openshift2, module_label, envoy, wildcard_d auth = AuthConfig.create_instance( openshift2, blame("ac"), None, hostnames=[wildcard_domain], labels={"testRun": module_label} ) - auth.responses.add({"name": "header", "json": {"properties": [{"name": "anything", "value": "two"}]}}) + auth.responses.json("header", [Property("anything", Value("two"))]) return auth diff --git a/testsuite/tests/kuadrant/authorino/operator/http/conftest.py b/testsuite/tests/kuadrant/authorino/operator/http/conftest.py index f0987d60..0f071644 100644 --- a/testsuite/tests/kuadrant/authorino/operator/http/conftest.py +++ b/testsuite/tests/kuadrant/authorino/operator/http/conftest.py @@ -1,6 +1,7 @@ """Conftest for all tests requiring custom deployment of Authorino""" import pytest +from testsuite.objects import Property, Value from testsuite.httpx import HttpxBackoffClient from testsuite.openshift.objects.auth_config import AuthConfig @@ -11,12 +12,9 @@ def authorization(authorization, wildcard_domain, openshift, module_label) -> Au """In case of Authorino, AuthConfig used for authorization""" authorization.remove_all_hosts() authorization.add_host(wildcard_domain) - resp = { - "name": "another-json-returned-in-a-header", - "wrapperKey": "x-ext-auth-other-json", - "json": {"properties": [{"name": "propX", "value": "valueX"}]}, - } - authorization.responses.add(response=resp) + authorization.responses.json( + "another-json-returned-in-a-header", [Property("propX", Value("valueX"))], wrapper_key="x-ext-auth-other-json" + ) return authorization diff --git a/testsuite/tests/kuadrant/authorino/operator/sharding/conftest.py b/testsuite/tests/kuadrant/authorino/operator/sharding/conftest.py index f4d9d129..579704bc 100644 --- a/testsuite/tests/kuadrant/authorino/operator/sharding/conftest.py +++ b/testsuite/tests/kuadrant/authorino/operator/sharding/conftest.py @@ -1,6 +1,7 @@ """Conftest for authorino sharding tests""" import pytest +from testsuite.objects import Property, Value from testsuite.openshift.envoy import Envoy from testsuite.openshift.objects.auth_config import AuthConfig @@ -31,7 +32,7 @@ def _authorization(hostname=None, sharding_label=None): hostnames=[hostname], labels={"testRun": module_label, "sharding": sharding_label}, ) - auth.responses.add({"name": "header", "json": {"properties": [{"name": "anything", "value": sharding_label}]}}) + auth.responses.json("header", [Property("anything", Value(sharding_label))]) request.addfinalizer(auth.delete) auth.commit() return auth diff --git a/testsuite/tests/kuadrant/authorino/wristband/conftest.py b/testsuite/tests/kuadrant/authorino/wristband/conftest.py index 72745ec7..db50c951 100644 --- a/testsuite/tests/kuadrant/authorino/wristband/conftest.py +++ b/testsuite/tests/kuadrant/authorino/wristband/conftest.py @@ -62,22 +62,7 @@ def wristband_endpoint(openshift, authorino, authorization_name): @pytest.fixture(scope="module") def authorization(authorization, wristband_secret, wristband_endpoint) -> AuthConfig: """Add wristband response with the signing key to the AuthConfig""" - authorization.responses.add( - { - "name": "wristband", - "wrapper": "envoyDynamicMetadata", - "wristband": { - "issuer": wristband_endpoint, - "tokenDuration": 300, # default value - "signingKeyRefs": [ - { - "name": wristband_secret, - "algorithm": "RS256", - } - ], - }, - } - ) + authorization.responses.wristband("wristband", wristband_endpoint, wristband_secret, wrapper="envoyDynamicMetadata") return authorization