From b6be53cf5811fec3fe0f3247678dcbafc6ed913b Mon Sep 17 00:00:00 2001 From: Serhii Lazebnyi Date: Thu, 30 Jan 2025 18:58:32 +0100 Subject: [PATCH 1/7] Added json.loads to jwt aithenticator params --- airbyte_cdk/sources/declarative/auth/jwt.py | 24 ++++++++++--------- .../sources/declarative/auth/test_jwt.py | 12 ++++++++++ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/airbyte_cdk/sources/declarative/auth/jwt.py b/airbyte_cdk/sources/declarative/auth/jwt.py index d7dd59282..35194d09e 100644 --- a/airbyte_cdk/sources/declarative/auth/jwt.py +++ b/airbyte_cdk/sources/declarative/auth/jwt.py @@ -2,6 +2,7 @@ # Copyright (c) 2023 Airbyte, Inc., all rights reserved. # +import json import base64 from dataclasses import InitVar, dataclass from datetime import datetime @@ -104,21 +105,21 @@ def __post_init__(self, parameters: Mapping[str, Any]) -> None: ) def _get_jwt_headers(self) -> dict[str, Any]: - """ " + """ Builds and returns the headers used when signing the JWT. """ - headers = self._additional_jwt_headers.eval(self.config) + headers = self._additional_jwt_headers.eval(self.config, json_loads=json.loads) if any(prop in headers for prop in ["kid", "alg", "typ", "cty"]): raise ValueError( "'kid', 'alg', 'typ', 'cty' are reserved headers and should not be set as part of 'additional_jwt_headers'" ) if self._kid: - headers["kid"] = self._kid.eval(self.config) + headers["kid"] = self._kid.eval(self.config, json_loads=json.loads) if self._typ: - headers["typ"] = self._typ.eval(self.config) + headers["typ"] = self._typ.eval(self.config, json_loads=json.loads) if self._cty: - headers["cty"] = self._cty.eval(self.config) + headers["cty"] = self._cty.eval(self.config, json_loads=json.loads) headers["alg"] = self._algorithm return headers @@ -130,18 +131,19 @@ def _get_jwt_payload(self) -> dict[str, Any]: exp = now + self._token_duration if isinstance(self._token_duration, int) else now nbf = now - payload = self._additional_jwt_payload.eval(self.config) + payload = self._additional_jwt_payload.eval(self.config, json_loads=json.loads) if any(prop in payload for prop in ["iss", "sub", "aud", "iat", "exp", "nbf"]): raise ValueError( "'iss', 'sub', 'aud', 'iat', 'exp', 'nbf' are reserved properties and should not be set as part of 'additional_jwt_payload'" ) if self._iss: - payload["iss"] = self._iss.eval(self.config) + payload["iss"] = self._iss.eval(self.config, json_loads=json.loads) if self._sub: - payload["sub"] = self._sub.eval(self.config) + payload["sub"] = self._sub.eval(self.config, json_loads=json.loads) if self._aud: - payload["aud"] = self._aud.eval(self.config) + payload["aud"] = self._aud.eval(self.config, json_loads=json.loads) + payload["iat"] = now payload["exp"] = exp payload["nbf"] = nbf @@ -151,7 +153,7 @@ def _get_secret_key(self) -> str: """ Returns the secret key used to sign the JWT. """ - secret_key: str = self._secret_key.eval(self.config) + secret_key: str = self._secret_key.eval(self.config, json_loads=json.loads) return ( base64.b64encode(secret_key.encode()).decode() if self._base64_encode_secret_key @@ -176,7 +178,7 @@ def _get_header_prefix(self) -> Union[str, None]: """ Returns the header prefix to be used when attaching the token to the request. """ - return self._header_prefix.eval(self.config) if self._header_prefix else None + return self._header_prefix.eval(self.config, json_loads=json.loads) if self._header_prefix else None @property def auth_header(self) -> str: diff --git a/unit_tests/sources/declarative/auth/test_jwt.py b/unit_tests/sources/declarative/auth/test_jwt.py index fe727b980..7030d6ab7 100644 --- a/unit_tests/sources/declarative/auth/test_jwt.py +++ b/unit_tests/sources/declarative/auth/test_jwt.py @@ -126,6 +126,18 @@ def test_get_secret_key(self, base64_encode_secret_key, secret_key, expected): ) assert authenticator._get_secret_key() == expected + def test_get_secret_key_from_config(self,): + authenticator = JwtAuthenticator( + config={'secrets': '{"secret_key": "test"}'}, + parameters={}, + secret_key="{{ json_loads(config['secrets'])['secret_key'] }}", + algorithm="test_algo", + token_duration=1200, + base64_encode_secret_key=False, + ) + expected = "test" + assert authenticator._get_secret_key() == expected + def test_get_signed_token(self): authenticator = JwtAuthenticator( config={}, From 2356d2e5b5f2c2ffae321b44b8697d3ef48eb0a9 Mon Sep 17 00:00:00 2001 From: Serhii Lazebnyi Date: Fri, 31 Jan 2025 14:38:27 +0100 Subject: [PATCH 2/7] Add validation for dynamic stream name --- .../manifest_declarative_source.py | 3 ++ .../test_http_components_resolver.py | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/airbyte_cdk/sources/declarative/manifest_declarative_source.py b/airbyte_cdk/sources/declarative/manifest_declarative_source.py index 78aeac23f..bd356f1a0 100644 --- a/airbyte_cdk/sources/declarative/manifest_declarative_source.py +++ b/airbyte_cdk/sources/declarative/manifest_declarative_source.py @@ -365,6 +365,9 @@ def _dynamic_stream_configs( # Ensure that each stream is created with a unique name name = dynamic_stream.get("name") + if not isinstance(name, str): + raise ValueError(f"Expected stream name {name} to be a string, got {type(name)}.") + if name in seen_dynamic_streams: error_message = f"Dynamic streams list contains a duplicate name: {name}. Please contact Airbyte Support." failure_type = FailureType.system_error diff --git a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py index f09ede0d6..43c09dd07 100644 --- a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py +++ b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py @@ -533,3 +533,37 @@ def test_dynamic_streams_with_http_components_resolver_retriever_with_parent_str actual_record_stream_names.sort() assert actual_record_stream_names == expected_stream_names + + +def test_wrong_stream_name_type(): + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/items"), + HttpResponse( + body=json.dumps( + [ + {"id": 1, "name": 1}, + {"id": 2, "name": 2}, + ] + ) + ), + ) + http_mocker.get( + HttpRequest(url="https://api.test.com/items/1"), + HttpResponse(body=json.dumps({"id": "1", "name": "item_1"})), + ) + http_mocker.get( + HttpRequest(url="https://api.test.com/items/2"), + HttpResponse(body=json.dumps({"id": "2", "name": "item_2"})), + ) + + source = ConcurrentDeclarativeSource( + source_config=_MANIFEST, config=_CONFIG, catalog=None, state=None + ) + with pytest.raises(ValueError) as exc_info: + source.discover(logger=source.logger, config=_CONFIG) + + assert ( + str(exc_info.value) + == "Expected stream name 1 to be a string, got ." + ) From 7948a96829d71ceb9a3fa2397701a8b54c900e05 Mon Sep 17 00:00:00 2001 From: octavia-squidington-iii Date: Fri, 31 Jan 2025 14:50:24 +0000 Subject: [PATCH 3/7] Auto-fix lint and format issues --- airbyte_cdk/sources/declarative/auth/jwt.py | 1 - .../sources/declarative/manifest_declarative_source.py | 4 +++- .../declarative/resolvers/test_http_components_resolver.py | 5 +---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/airbyte_cdk/sources/declarative/auth/jwt.py b/airbyte_cdk/sources/declarative/auth/jwt.py index 0138ed67a..c83d081bb 100644 --- a/airbyte_cdk/sources/declarative/auth/jwt.py +++ b/airbyte_cdk/sources/declarative/auth/jwt.py @@ -2,7 +2,6 @@ # Copyright (c) 2023 Airbyte, Inc., all rights reserved. # -import json import base64 import json from dataclasses import InitVar, dataclass diff --git a/airbyte_cdk/sources/declarative/manifest_declarative_source.py b/airbyte_cdk/sources/declarative/manifest_declarative_source.py index bd356f1a0..efc779464 100644 --- a/airbyte_cdk/sources/declarative/manifest_declarative_source.py +++ b/airbyte_cdk/sources/declarative/manifest_declarative_source.py @@ -366,7 +366,9 @@ def _dynamic_stream_configs( name = dynamic_stream.get("name") if not isinstance(name, str): - raise ValueError(f"Expected stream name {name} to be a string, got {type(name)}.") + raise ValueError( + f"Expected stream name {name} to be a string, got {type(name)}." + ) if name in seen_dynamic_streams: error_message = f"Dynamic streams list contains a duplicate name: {name}. Please contact Airbyte Support." diff --git a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py index 43c09dd07..fb4d0b6df 100644 --- a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py +++ b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py @@ -563,7 +563,4 @@ def test_wrong_stream_name_type(): with pytest.raises(ValueError) as exc_info: source.discover(logger=source.logger, config=_CONFIG) - assert ( - str(exc_info.value) - == "Expected stream name 1 to be a string, got ." - ) + assert str(exc_info.value) == "Expected stream name 1 to be a string, got ." From 9eefefaacece6c1ab21c8a1719d90444179c7d83 Mon Sep 17 00:00:00 2001 From: Serhii Lazebnyi Date: Fri, 31 Jan 2025 16:14:02 +0100 Subject: [PATCH 4/7] Add clear_all_matchers --- .../declarative/resolvers/test_http_components_resolver.py | 1 + 1 file changed, 1 insertion(+) diff --git a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py index fb4d0b6df..c50a45448 100644 --- a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py +++ b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py @@ -537,6 +537,7 @@ def test_dynamic_streams_with_http_components_resolver_retriever_with_parent_str def test_wrong_stream_name_type(): with HttpMocker() as http_mocker: + http_mocker.clear_all_matchers() http_mocker.get( HttpRequest(url="https://api.test.com/items"), HttpResponse( From f2373922b577a56f6e47b6ac95cde73c8046cc42 Mon Sep 17 00:00:00 2001 From: Serhii Lazebnyi Date: Fri, 31 Jan 2025 16:21:41 +0100 Subject: [PATCH 5/7] Remove temp solution --- .../test_http_components_resolver.py | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py index c50a45448..775504e95 100644 --- a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py +++ b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py @@ -362,6 +362,37 @@ def test_http_components_resolver( assert result == expected_result +def test_wrong_stream_name_type(): + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/items"), + HttpResponse( + body=json.dumps( + [ + {"id": 1, "name": 1}, + {"id": 2, "name": 2}, + ] + ) + ), + ) + http_mocker.get( + HttpRequest(url="https://api.test.com/items/1"), + HttpResponse(body=json.dumps({"id": "1", "name": "item_1"})), + ) + http_mocker.get( + HttpRequest(url="https://api.test.com/items/2"), + HttpResponse(body=json.dumps({"id": "2", "name": "item_2"})), + ) + + source = ConcurrentDeclarativeSource( + source_config=_MANIFEST, config=_CONFIG, catalog=None, state=None + ) + with pytest.raises(ValueError) as exc_info: + source.discover(logger=source.logger, config=_CONFIG) + + assert str(exc_info.value) == "Expected stream name 1 to be a string, got ." + + @pytest.mark.parametrize( "components_mapping, retriever_data, stream_template_config, expected_result", [ @@ -533,35 +564,3 @@ def test_dynamic_streams_with_http_components_resolver_retriever_with_parent_str actual_record_stream_names.sort() assert actual_record_stream_names == expected_stream_names - - -def test_wrong_stream_name_type(): - with HttpMocker() as http_mocker: - http_mocker.clear_all_matchers() - http_mocker.get( - HttpRequest(url="https://api.test.com/items"), - HttpResponse( - body=json.dumps( - [ - {"id": 1, "name": 1}, - {"id": 2, "name": 2}, - ] - ) - ), - ) - http_mocker.get( - HttpRequest(url="https://api.test.com/items/1"), - HttpResponse(body=json.dumps({"id": "1", "name": "item_1"})), - ) - http_mocker.get( - HttpRequest(url="https://api.test.com/items/2"), - HttpResponse(body=json.dumps({"id": "2", "name": "item_2"})), - ) - - source = ConcurrentDeclarativeSource( - source_config=_MANIFEST, config=_CONFIG, catalog=None, state=None - ) - with pytest.raises(ValueError) as exc_info: - source.discover(logger=source.logger, config=_CONFIG) - - assert str(exc_info.value) == "Expected stream name 1 to be a string, got ." From b695b766113770b1df2207856ab027c021a4d499 Mon Sep 17 00:00:00 2001 From: Serhii Lazebnyi Date: Fri, 31 Jan 2025 16:42:14 +0100 Subject: [PATCH 6/7] Update path for component resolver --- .../resolvers/test_http_components_resolver.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py index 775504e95..a325b5646 100644 --- a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py +++ b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py @@ -6,7 +6,7 @@ from unittest.mock import MagicMock import pytest - +from copy import deepcopy from airbyte_cdk.models import Type from airbyte_cdk.sources.declarative.concurrent_declarative_source import ( ConcurrentDeclarativeSource, @@ -365,7 +365,7 @@ def test_http_components_resolver( def test_wrong_stream_name_type(): with HttpMocker() as http_mocker: http_mocker.get( - HttpRequest(url="https://api.test.com/items"), + HttpRequest(url="https://api.test.com/int_items"), HttpResponse( body=json.dumps( [ @@ -375,17 +375,12 @@ def test_wrong_stream_name_type(): ) ), ) - http_mocker.get( - HttpRequest(url="https://api.test.com/items/1"), - HttpResponse(body=json.dumps({"id": "1", "name": "item_1"})), - ) - http_mocker.get( - HttpRequest(url="https://api.test.com/items/2"), - HttpResponse(body=json.dumps({"id": "2", "name": "item_2"})), - ) + + manifest = deepcopy(_MANIFEST) + manifest["dynamic_streams"][0]["components_resolver"]["retriever"]["requester"]["path"] = "int_items" source = ConcurrentDeclarativeSource( - source_config=_MANIFEST, config=_CONFIG, catalog=None, state=None + source_config=manifest, config=_CONFIG, catalog=None, state=None ) with pytest.raises(ValueError) as exc_info: source.discover(logger=source.logger, config=_CONFIG) From cd4d89dcccc5915329664bd8ce251154f5a61d88 Mon Sep 17 00:00:00 2001 From: octavia-squidington-iii Date: Fri, 31 Jan 2025 15:43:49 +0000 Subject: [PATCH 7/7] Auto-fix lint and format issues --- .../declarative/resolvers/test_http_components_resolver.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py index a325b5646..357dcceef 100644 --- a/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py +++ b/unit_tests/sources/declarative/resolvers/test_http_components_resolver.py @@ -3,10 +3,11 @@ # import json +from copy import deepcopy from unittest.mock import MagicMock import pytest -from copy import deepcopy + from airbyte_cdk.models import Type from airbyte_cdk.sources.declarative.concurrent_declarative_source import ( ConcurrentDeclarativeSource, @@ -377,7 +378,9 @@ def test_wrong_stream_name_type(): ) manifest = deepcopy(_MANIFEST) - manifest["dynamic_streams"][0]["components_resolver"]["retriever"]["requester"]["path"] = "int_items" + manifest["dynamic_streams"][0]["components_resolver"]["retriever"]["requester"]["path"] = ( + "int_items" + ) source = ConcurrentDeclarativeSource( source_config=manifest, config=_CONFIG, catalog=None, state=None