From 29fb89a1362f752d87957634775967637b648015 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 19 Aug 2026 13:29:52 +0300 Subject: [PATCH 01/10] feat(low-code): let the check component override config values A check and a sync legitimately want different behaviour from the same manifest. A check is interactive and should fail fast with an actionable message; a sync can afford to wait out a rate limit window. Today the only way to express that difference is a Python `check_connection` override that builds a second component tree from a modified config, which a manifest-only connector cannot do. `CheckStream` and `CheckDynamicStream` gain an optional `config_overrides` mapping. `check()` overlays it onto the config for the duration of the check, which is enough to reach every component the checker builds because `streams()` interpolates from `self._config` and ignores its own `config` argument. Values are applied verbatim and are not interpolated. `config_validations` continue to run against the config the user supplied, so an override cannot fail a validation the user has no way to satisfy. Inert by default: with no `config_overrides` key, `check()` behaves exactly as before. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 38 +++- .../declarative_component_schema.yaml | 16 ++ .../models/declarative_component_schema.py | 12 ++ .../declarative/checks/test_check_stream.py | 183 ++++++++++++++++++ 4 files changed, 247 insertions(+), 2 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index d7e5e4260..e6faa929b 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -3,6 +3,7 @@ import json import logging import pkgutil +from contextlib import contextmanager from copy import deepcopy from dataclasses import dataclass, field from queue import Queue @@ -228,6 +229,10 @@ def __init__( self._constructor.create_component(SpecModel, spec, dict()) if spec else None ) self._config = self._migrate_and_transform_config(config_path, config) or {} + # `check` may temporarily overlay values onto `self._config` (see + # `_config_overridden_for_check`). The manifest's `config_validations` express intent about what + # the *user* supplied, so they must always run against the unmodified config. + self._user_provided_config = self._config concurrency_level_from_manifest = self._source_config.get("concurrency_level") if concurrency_level_from_manifest: @@ -413,7 +418,7 @@ def streams(self, config: Mapping[str, Any]) -> List[AbstractStream]: # type: i """ if self._spec_component: - self._spec_component.validate_config(self._config) + self._spec_component.validate_config(self._user_provided_config) api_budget_model = self._source_config.get("api_budget") if api_budget_model: @@ -607,11 +612,40 @@ def check(self, logger: logging.Logger, config: Mapping[str, Any]) -> AirbyteCon f"Expected to generate a ConnectionChecker component, but received {connection_checker.__class__}" ) - check_succeeded, error = connection_checker.check_connection(self, logger, self._config) + with self._config_overridden_for_check(check.get("config_overrides")): + check_succeeded, error = connection_checker.check_connection(self, logger, self._config) if not check_succeeded: return AirbyteConnectionStatus(status=Status.FAILED, message=repr(error)) return AirbyteConnectionStatus(status=Status.SUCCEEDED) + @contextmanager + def _config_overridden_for_check( + self, config_overrides: Optional[Mapping[str, Any]] + ) -> Iterator[None]: + """Overlay the check component's `config_overrides` onto the config for the duration of a check. + + A check and a sync legitimately want different behaviour from the same manifest: a check is + interactive and should fail fast with an actionable message, while a sync can afford to wait out + a rate limit window. Before this existed, expressing that difference required a Python + `check_connection` override that built a second component tree from a modified config, which is + not available to a manifest-only connector. + + Overlaying here is enough to reach every component the checker builds, because `streams()` + interpolates from `self._config` and ignores its own `config` argument. Values are applied + verbatim - they are not interpolated - and `config_validations` still run against the config the + user supplied, so an override cannot fail a validation the user has no way to satisfy. + """ + if not config_overrides: + yield + return + + unmodified_config = self._config + self._config = {**self._config, **config_overrides} + try: + yield + finally: + self._config = unmodified_config + @property def dynamic_streams(self) -> List[Dict[str, Any]]: return self._dynamic_stream_configs( diff --git a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml index 091c694d2..ebedd71b7 100644 --- a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml +++ b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml @@ -549,6 +549,14 @@ definitions: type: array items: "$ref": "#/definitions/DynamicStreamCheckConfig" + config_overrides: + title: Config Overrides + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated. + type: object + additionalProperties: true + examples: + - max_waiting_time: 0 + - page_size: 1 DynamicStreamCheckConfig: type: object required: @@ -587,6 +595,14 @@ definitions: description: Enables stream check availability. This field is automatically set by the CDK. type: boolean default: true + config_overrides: + title: Config Overrides + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated. + type: object + additionalProperties: true + examples: + - max_waiting_time: 0 + - page_size: 1 CompositeErrorHandler: title: Composite Error Handler description: Error handler that sequentially iterates over a list of error handlers. diff --git a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py index 37eb50b7b..b20cdb4e4 100644 --- a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py +++ b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py @@ -77,6 +77,12 @@ class CheckDynamicStream(BaseModel): description="Enables stream check availability. This field is automatically set by the CDK.", title="Use Check Availability", ) + config_overrides: Optional[Dict[str, Any]] = Field( + None, + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated.", + examples=[{"max_waiting_time": 0}, {"page_size": 1}], + title="Config Overrides", + ) class ConcurrencyLevel(BaseModel): @@ -1781,6 +1787,12 @@ class CheckStream(BaseModel): title="Stream Names", ) dynamic_streams_check_configs: Optional[List[DynamicStreamCheckConfig]] = None + config_overrides: Optional[Dict[str, Any]] = Field( + None, + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated.", + examples=[{"max_waiting_time": 0}, {"page_size": 1}], + title="Config Overrides", + ) class IncrementingCountCursor(BaseModel): diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index 8bc9571d1..9a488de5a 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -907,3 +907,186 @@ def test_check_stream_only_type_provided(): ) with pytest.raises(ValueError): source.check(logger, _CONFIG) + + +_CONFIG_DRIVEN_PATH_CONFIG = {"resource": "sync"} + +_MANIFEST_WITH_CONFIG_DRIVEN_PATH = { + "version": "6.7.0", + "type": "DeclarativeSource", + "check": {"type": "CheckStream", "stream_names": ["items"]}, + "streams": [ + { + "type": "DeclarativeStream", + "name": "items", + "primary_key": "id", + "schema_loader": { + "type": "InlineSchemaLoader", + "schema": { + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": {"id": {"type": "integer"}}, + }, + }, + "retriever": { + "type": "SimpleRetriever", + "requester": { + "type": "HttpRequester", + "url": "https://api.test.com/{{ config['resource'] }}", + "http_method": "GET", + }, + "record_selector": { + "type": "RecordSelector", + "extractor": {"type": "DpathExtractor", "field_path": []}, + }, + "paginator": {"type": "NoPagination"}, + }, + } + ], +} + + +def _source_with_check_component(check_component): + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = check_component + return ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + +def test_given_no_config_overrides_when_check_then_components_use_the_user_config(): + source = _source_with_check_component({"type": "CheckStream", "stream_names": ["items"]}) + + with HttpMocker() as http_mocker: + # Only the user-configured path is mocked, so a request to any other path fails the test. + http_mocker.get( + HttpRequest(url="https://api.test.com/sync"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_config_overrides_when_check_then_components_built_during_check_see_them(): + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + + with HttpMocker() as http_mocker: + # Only the overridden path is mocked. Reaching this endpoint proves the overlay was applied to + # the stream the checker built, and not merely stored on the source. + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(overridden_request, 1) + + +def test_given_config_overrides_when_check_then_the_config_is_restored_afterwards(): + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert source._config == _CONFIG_DRIVEN_PATH_CONFIG + + # A sync that follows a check in the same process must go back to the user's value. + with HttpMocker() as http_mocker: + sync_request = HttpRequest(url="https://api.test.com/sync") + http_mocker.get(sync_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + stream = source.streams(_CONFIG_DRIVEN_PATH_CONFIG)[0] + assert stream.check_availability().is_available + + http_mocker.assert_number_of_calls(sync_request, 1) + + +def test_given_config_overrides_when_check_raises_then_the_config_is_restored(): + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["not_in_the_catalog"], + "config_overrides": {"resource": "check-only"}, + } + ) + + with pytest.raises(ValueError): + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + + assert source._config == _CONFIG_DRIVEN_PATH_CONFIG + + +def test_given_config_overrides_when_check_then_config_validations_run_against_the_user_config(): + """An override is authored in the manifest, so it must not be held to validations written for the + user's own input - the user has no way to satisfy them.""" + config = {"resource": "sync", "settings": {"mode": "sync"}} + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only", "settings": {"mode": "check-only"}}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "resource": {"type": "string"}, + "settings": {"type": "object"}, + }, + }, + "config_normalization_rules": { + "type": "ConfigNormalizationRules", + "validations": [ + { + "type": "DpathValidator", + "field_path": ["settings"], + "validation_strategy": { + "type": "ValidateAdheresToSchema", + "base_schema": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {"mode": {"type": "string", "enum": ["sync"]}}, + "required": ["mode"], + "additionalProperties": False, + }, + }, + } + ], + }, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=config, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + # `settings.mode` is overridden to a value outside the validator's enum, so this would fail were + # the overlay validated instead of the config the user supplied. + assert source.check(logger, config).status == Status.SUCCEEDED From 0e23e0479eb83025fed3008d248e67f342379eee Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 19 Aug 2026 14:09:00 +0300 Subject: [PATCH 02/10] fix(low-code): document and pin config_overrides semantics Follow-up on the same branch, addressing a local review of the feature. Documents three semantics the field description and docstring left implicit: overrides are merged one level deep so a nested object is replaced rather than deep-merged; they are applied after config migrations and transformations, so an override is not normalised and derived fields are not recomputed; and the overlay mutates shared state, which is safe only because check is one command per process. Renames `_user_provided_config` to `_config_for_validation`. It holds the config after migrations and transformations have run, not what the user typed, and the old name plus its comment invited the wrong reading. Adds tests for two contracts that were stated but unpinned: `CheckDynamicStream` is covered by the overlay, and override values are verbatim rather than interpolated. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 28 ++++-- .../declarative_component_schema.yaml | 4 +- .../models/declarative_component_schema.py | 4 +- .../declarative/checks/test_check_stream.py | 99 +++++++++++++++++++ 4 files changed, 124 insertions(+), 11 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index e6faa929b..5d67501e0 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -230,9 +230,9 @@ def __init__( ) self._config = self._migrate_and_transform_config(config_path, config) or {} # `check` may temporarily overlay values onto `self._config` (see - # `_config_overridden_for_check`). The manifest's `config_validations` express intent about what - # the *user* supplied, so they must always run against the unmodified config. - self._user_provided_config = self._config + # `_config_overridden_for_check`). The manifest's `config_validations` express intent about the + # config as supplied, so they must run against it rather than against a check-time overlay. + self._config_for_validation = self._config concurrency_level_from_manifest = self._source_config.get("concurrency_level") if concurrency_level_from_manifest: @@ -418,7 +418,7 @@ def streams(self, config: Mapping[str, Any]) -> List[AbstractStream]: # type: i """ if self._spec_component: - self._spec_component.validate_config(self._user_provided_config) + self._spec_component.validate_config(self._config_for_validation) api_budget_model = self._source_config.get("api_budget") if api_budget_model: @@ -631,9 +631,23 @@ def _config_overridden_for_check( not available to a manifest-only connector. Overlaying here is enough to reach every component the checker builds, because `streams()` - interpolates from `self._config` and ignores its own `config` argument. Values are applied - verbatim - they are not interpolated - and `config_validations` still run against the config the - user supplied, so an override cannot fail a validation the user has no way to satisfy. + interpolates from `self._config` and ignores its own `config` argument. + + Semantics, all deliberate: + - Values are applied verbatim. They are not interpolated, so a value containing `{{ }}` reaches + components as that literal string. + - The merge is one level deep. Overriding a key whose value is an object replaces that object + rather than merging into it, which keeps removing a nested key expressible. + - The overlay happens long after `_migrate_and_transform_config`, so an override is neither + normalised itself nor propagated to fields derived from it by a `ConfigTransformation`. + - `config_validations` run against `self._config_for_validation`, so an override cannot fail a + validation written for the config as supplied. + - `self._config` is shared state. Reassigning it is not thread safe, and a component that writes + back into the config during check - a `SingleUseRefreshTokenOauth2Authenticator` refreshing its + token, say - writes into the throwaway overlay, which the restore then discards. The control + message is still emitted so the platform persists the new token, but an in-process read + following a check would carry the stale one. Both are acceptable while this is scoped to + `check`, which is one command per process. """ if not config_overrides: yield diff --git a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml index ebedd71b7..7506806b4 100644 --- a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml +++ b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml @@ -551,7 +551,7 @@ definitions: "$ref": "#/definitions/DynamicStreamCheckConfig" config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. type: object additionalProperties: true examples: @@ -597,7 +597,7 @@ definitions: default: true config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. type: object additionalProperties: true examples: diff --git a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py index b20cdb4e4..803e820c1 100644 --- a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py +++ b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py @@ -79,7 +79,7 @@ class CheckDynamicStream(BaseModel): ) config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) @@ -1789,7 +1789,7 @@ class CheckStream(BaseModel): dynamic_streams_check_configs: Optional[List[DynamicStreamCheckConfig]] = None config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is and are not interpolated.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index 9a488de5a..bbc978f31 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -1090,3 +1090,102 @@ def test_given_config_overrides_when_check_then_config_validations_run_against_t # `settings.mode` is overridden to a value outside the validator's enum, so this would fail were # the overlay validated instead of the config the user supplied. assert source.check(logger, config).status == Status.SUCCEEDED + + +def test_given_config_overrides_when_check_then_values_are_not_interpolated(): + """Pins the verbatim contract. A value containing `{{ }}` reaches components as that literal string, + so turning interpolation on later is a deliberate, test-breaking decision rather than a silent + reinterpretation of overrides already written.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "{{config['resource']}}"}, + } + ) + + with HttpMocker() as http_mocker: + literal_request = HttpRequest(url="https://api.test.com/%7B%7Bconfig%5B'resource'%5D%7D%7D") + http_mocker.get(literal_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(literal_request, 1) + + +_MANIFEST_WITH_CONFIG_DRIVEN_DYNAMIC_STREAM = { + "version": "6.7.0", + "type": "DeclarativeSource", + "check": {"type": "CheckDynamicStream", "stream_count": 1}, + "streams": [], + "dynamic_streams": [ + { + "type": "DynamicDeclarativeStream", + "name": "dynamic_items", + "stream_template": { + "type": "DeclarativeStream", + "name": "", + "primary_key": [], + "schema_loader": { + "type": "InlineSchemaLoader", + "schema": { + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": {"id": {"type": "integer"}}, + }, + }, + "retriever": { + "type": "SimpleRetriever", + "requester": { + "type": "HttpRequester", + "url": "https://api.test.com/{{ config['resource'] }}", + "http_method": "GET", + }, + "record_selector": { + "type": "RecordSelector", + "extractor": {"type": "DpathExtractor", "field_path": []}, + }, + "paginator": {"type": "NoPagination"}, + }, + }, + "components_resolver": { + "type": "ConfigComponentsResolver", + "stream_config": { + "type": "StreamConfig", + "configs_pointer": ["custom_streams"], + }, + "components_mapping": [ + { + "type": "ComponentMappingDefinition", + "field_path": ["name"], + "value": "{{components_values['name']}}", + } + ], + }, + } + ], +} + + +def test_given_config_overrides_on_check_dynamic_stream_then_components_see_them(): + """The overlay is read from the raw check definition, so it is checker-agnostic. Without this test a + refactor moving the read into `create_check_stream` would silently drop `CheckDynamicStream`.""" + config = {"resource": "sync", "custom_streams": [{"name": "items"}]} + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_DYNAMIC_STREAM) + manifest["check"] = { + "type": "CheckDynamicStream", + "stream_count": 1, + "config_overrides": {"resource": "check-only"}, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=config, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, config).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(overridden_request, 1) From 9b1c62bf88b44ed521ed56b84c19dd57db26e2c8 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 19 Aug 2026 18:35:09 +0300 Subject: [PATCH 03/10] fix(low-code): address review feedback on config_overrides Refuses `config_overrides` on a manifest that declares a `refresh_token_updater`. That turns an `OAuthAuthenticator` into a `DeclarativeSingleUseRefreshTokenOauth2Authenticator`, which is handed the config the component tree was built with - during a check, the overlay - and on refresh emits that entire dict as a CONNECTOR_CONFIG control message for the platform to persist. A check-only override would therefore become the connection's saved config and apply to every later sync, and restoring `self._config` afterwards cannot recall a message already written to stdout. Threading the config through more carefully would not help: the hazard is inherent to handing an overridden config to something whose job is to write the config back. Until the emitter is fixed to emit the config it was given plus only the token fields it owns, refusing the combination is the honest answer. Refuses override keys prefixed with `__airbyte`. Those are the platform's channel into the config rather than connector config - `CheckStream` reads `__airbyte_check_stream_names` out of the very config this overlay writes to - and a manifest that wants to choose which streams a check tests already has `stream_names`. Logs the overridden keys at INFO, keys only, since an override may name a secret field. Warns when an override key is absent from the spec's `connection_specification.properties`: the overlay is the one part of the config nothing validates, so a typo is otherwise a silent no-op. Corrects the shallow-copy paragraph in the docstring, which had it backwards. Because the copy is shallow, every nested object is shared with the config the source was constructed with, so a write into a nested path such as `("credentials", "access_token")` writes through to the user's config and the restore does not undo it. Only a write to a top-level key is discarded. Notes in `create_check_stream` and `create_check_dynamic_stream` that `model.config_overrides` is deliberately unread there because the source applies it around the whole check operation, so nobody wires it in twice or deletes it as dead. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 104 +++++++++++++- .../parsers/model_to_component_factory.py | 4 + .../declarative/checks/test_check_stream.py | 135 ++++++++++++++++++ 3 files changed, 237 insertions(+), 6 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index 5d67501e0..fbdaa4c57 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -642,17 +642,31 @@ def _config_overridden_for_check( normalised itself nor propagated to fields derived from it by a `ConfigTransformation`. - `config_validations` run against `self._config_for_validation`, so an override cannot fail a validation written for the config as supplied. - - `self._config` is shared state. Reassigning it is not thread safe, and a component that writes - back into the config during check - a `SingleUseRefreshTokenOauth2Authenticator` refreshing its - token, say - writes into the throwaway overlay, which the restore then discards. The control - message is still emitted so the platform persists the new token, but an in-process read - following a check would carry the stale one. Both are acceptable while this is scoped to - `check`, which is one command per process. + + Two consequences of the copy being shallow and of `self._config` being shared state: + - Rebinding `self._config` is not thread safe. Acceptable only because `check` is one command + per process. + - The copy is shallow, so every nested object is the *same* object as in the config the source + was constructed with. A component that writes into a nested path - `dpath.new(config, + ("credentials", "access_token"), ...)` - therefore writes through to that shared dict, and the + restore does not undo it, because the restore only rebinds the top level. Only a write to a + top-level key is discarded. + + The write-through above is why a manifest that declares a `refresh_token_updater` is rejected + outright rather than documented: see `_raise_if_config_is_persisted`. """ if not config_overrides: yield return + self._raise_on_reserved_override_keys(config_overrides) + self._raise_if_config_is_persisted(config_overrides) + self._warn_on_unknown_override_keys(config_overrides) + # Keys only. An override may name a secret field, so values must not reach the logs. + self.logger.info( + f"Overriding config keys for the check operation: {', '.join(sorted(config_overrides))}" + ) + unmodified_config = self._config self._config = {**self._config, **config_overrides} try: @@ -660,6 +674,84 @@ def _config_overridden_for_check( finally: self._config = unmodified_config + @staticmethod + def _raise_on_reserved_override_keys(config_overrides: Mapping[str, Any]) -> None: + """Refuse to overlay keys in the platform's reserved `__airbyte` namespace. + + These are not connector config, they are the platform's channel into it: `CheckStream` reads + `__airbyte_check_stream_names` out of the very config this overlay writes to, so an override there + would change which streams a check tests. A manifest wanting that has `stream_names` for it. The + prefix is refused wholesale so future internal keys are covered too. + """ + reserved = sorted(key for key in config_overrides if key.startswith("__airbyte")) + if reserved: + raise ValueError( + f"`config_overrides` may not set the reserved key(s) {reserved}. Keys prefixed with " + "`__airbyte` belong to the platform, not to the connector's spec." + ) + + def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> None: + """Reject `config_overrides` on a manifest whose authenticator writes the config back. + + A `refresh_token_updater` turns an `OAuthAuthenticator` into a + `DeclarativeSingleUseRefreshTokenOauth2Authenticator`, which is handed the config the component + tree was built with - during a check, the overlay. When it refreshes, `_emit_control_message` + prints that *entire* dict as a `CONNECTOR_CONFIG` control message, and the platform persists it. + So a check-only override would become the connection's saved config and apply to every later sync. + + Restoring `self._config` afterwards cannot help: the control message is already on stdout. And no + amount of threading the config through properly would help either, because the hazard is inherent + to handing an overridden config to something whose job is to write the config back. Until the + emitter is fixed to emit the config it was given plus only the token fields it owns, refusing the + combination is the honest answer. + """ + if not self._manifest_writes_back_config(self._source_config): + return + raise ValueError( + "`config_overrides` cannot be used by a manifest that declares a `refresh_token_updater`. " + "A token refresh during `check` emits the whole config it was handed as a CONNECTOR_CONFIG " + "control message, which the platform persists, so the check-only " + f"override(s) {sorted(config_overrides)} would be saved as this connection's config and " + "applied to every later sync. Remove `config_overrides`, or drop the `refresh_token_updater`." + ) + + @staticmethod + def _manifest_writes_back_config(definition: Any) -> bool: + """Whether any component in the manifest emits the connector config back to the platform.""" + if isinstance(definition, Mapping): + if definition.get("refresh_token_updater"): + return True + return any( + ConcurrentDeclarativeSource._manifest_writes_back_config(value) + for value in definition.values() + ) + if isinstance(definition, list): + return any( + ConcurrentDeclarativeSource._manifest_writes_back_config(item) + for item in definition + ) + return False + + def _warn_on_unknown_override_keys(self, config_overrides: Mapping[str, Any]) -> None: + """Warn about override keys the spec does not declare. + + The overlay is the one part of the config nothing validates - the entrypoint validates what the + user supplied, and `config_validations` deliberately run against `self._config_for_validation`. So + a typo or a since-renamed field is a silent no-op that surfaces much later as "why does check no + longer fail fast". + """ + if not self._spec_component: + return + declared = self._spec_component.connection_specification.get("properties") + if not isinstance(declared, Mapping): + return + unknown = sorted(key for key in config_overrides if key not in declared) + if unknown: + self.logger.warning( + f"Check-only config override(s) {unknown} are not declared in the connector spec, so " + "they will have no effect on any component that reads the config by field name." + ) + @property def dynamic_streams(self) -> List[Dict[str, Any]]: return self._dynamic_stream_configs( diff --git a/airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py b/airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py index a96c406d8..1aa729d18 100644 --- a/airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py +++ b/airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py @@ -1306,6 +1306,9 @@ def create_check_stream( else [] ) + # `model.config_overrides` is deliberately not read here. The source applies it around the whole + # check operation (`ConcurrentDeclarativeSource._config_overridden_for_check`), which is what makes + # it work for every checker type rather than only this one. Do not wire it in a second time. return CheckStream( stream_names=model.stream_names or [], dynamic_streams_check_configs=dynamic_streams_check_configs, @@ -1320,6 +1323,7 @@ def create_check_dynamic_stream( use_check_availability = model.use_check_availability + # See `create_check_stream`: `model.config_overrides` is applied by the source, not here. return CheckDynamicStream( stream_count=model.stream_count, use_check_availability=use_check_availability, diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index bbc978f31..5c46c1bdd 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -1189,3 +1189,138 @@ def test_given_config_overrides_on_check_dynamic_stream_then_components_see_them assert source.check(logger, config).status == Status.SUCCEEDED http_mocker.assert_number_of_calls(overridden_request, 1) + + +_OAUTH_WITH_REFRESH_TOKEN_UPDATER = { + "type": "OAuthAuthenticator", + "token_refresh_endpoint": "https://api.test.com/oauth/token", + "client_id": "{{ config['credentials']['client_id'] }}", + "client_secret": "{{ config['credentials']['client_secret'] }}", + "refresh_token": "{{ config['credentials']['refresh_token'] }}", + "refresh_token_updater": {"type": "RefreshTokenUpdater", "refresh_token_name": "refresh_token"}, +} + + +def _manifest_with_refresh_token_updater(check_component): + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = check_component + manifest["streams"][0]["retriever"]["requester"]["authenticator"] = deepcopy( + _OAUTH_WITH_REFRESH_TOKEN_UPDATER + ) + return manifest + + +def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_rejected(): + """A `refresh_token_updater` emits the whole config it was handed as a CONNECTOR_CONFIG control + message, which the platform persists - so a check-only override would become the connection's saved + config. The restore cannot recall a message already on stdout, so the combination is refused.""" + source = ConcurrentDeclarativeSource( + source_config=_manifest_with_refresh_token_updater( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ), + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with pytest.raises(ValueError, match="refresh_token_updater"): + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + + +def test_given_refresh_token_updater_without_config_overrides_then_nothing_is_rejected(): + """The rejection is scoped to the feature. A manifest that does not use `config_overrides` keeps + working with a `refresh_token_updater` exactly as before, even though the manifest scan detects it.""" + source = ConcurrentDeclarativeSource( + source_config=_manifest_with_refresh_token_updater( + {"type": "CheckStream", "stream_names": ["items"]} + ), + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source.resolved_manifest) is True + + # No overrides means no overlay, so the guard must not fire and the config must not be copied. + with source._config_overridden_for_check(None): + assert source._config is _CONFIG_DRIVEN_PATH_CONFIG + + +def test_given_config_overrides_when_check_then_overridden_keys_are_logged_without_values(caplog): + """An override may name a secret field, so the log records which keys were overridden but never what + they were set to.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "s3cr3t-value"}, + } + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/s3cr3t-value"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + with caplog.at_level(logging.INFO): + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + + assert "Overriding config keys for the check operation: resource" in caplog.text + assert "s3cr3t-value" not in caplog.text + + +def test_given_override_key_absent_from_the_spec_then_a_warning_is_logged(caplog): + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only", "typoed_key": 1}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {"resource": {"type": "string"}}, + }, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + with caplog.at_level(logging.WARNING): + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + + assert "typoed_key" in caplog.text + assert "not declared in the connector spec" in caplog.text + assert "'resource'" not in caplog.text + + +def test_given_an_airbyte_reserved_override_key_then_the_manifest_is_rejected(): + """`__airbyte`-prefixed keys are the platform's channel into the config - `CheckStream` reads + `__airbyte_check_stream_names` from the very config the overlay writes to. The feature refuses to + touch that namespace rather than leaving it available.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"__airbyte_check_stream_names": ["something_else"]}, + } + ) + + with pytest.raises(ValueError, match="__airbyte_check_stream_names"): + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) From ae804ed56d270040d11f364b698f789a9e68cb38 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Thu, 20 Aug 2026 13:43:53 +0300 Subject: [PATCH 04/10] fix(low-code): detect refresh_token_updater taking all defaults `_manifest_writes_back_config` tested `refresh_token_updater` for truthiness. Every field of `RefreshTokenUpdater` has a default and none are required, so `refresh_token_updater: {}` is a valid way to take all of them - and it builds the same `DeclarativeSingleUseRefreshTokenOauth2Authenticator` a populated one does, because the factory's `if model.refresh_token_updater:` tests a model instance, which is always truthy. The transformer injects no `type` into it either, so the empty mapping stayed empty and slipped past the guard, leaving the config-persistence hazard open on exactly the shape the review reproduced. Testing `is not None` matches the factory on all three shapes: `{}`, populated, and absent. The rejection test is parametrized over the first two, and a new test covers the same OAuth authenticator without the updater, so the scan is pinned as not rejecting every OAuth manifest. Also document both rejections in the `config_overrides` description on `CheckStream` and `CheckDynamicStream`, since neither the `__airbyte` prefix restriction nor the `refresh_token_updater` one was discoverable before running a check. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 11 +++- .../declarative_component_schema.yaml | 4 +- .../models/declarative_component_schema.py | 4 +- .../declarative/checks/test_check_stream.py | 50 ++++++++++++++++--- 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index fbdaa4c57..bbb54e4b0 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -717,9 +717,16 @@ def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> @staticmethod def _manifest_writes_back_config(definition: Any) -> bool: - """Whether any component in the manifest emits the connector config back to the platform.""" + """Whether any component in the manifest emits the connector config back to the platform. + + Tested with `is not None` rather than for truthiness, to match the factory. Every field of + `RefreshTokenUpdater` has a default, so `refresh_token_updater: {}` is a valid way to take all of + them - and it builds a single-use authenticator just like a populated one, because the factory's + `if model.refresh_token_updater:` sees a model instance, which is always truthy. A truthiness test + here would see an empty dict and let that manifest through. + """ if isinstance(definition, Mapping): - if definition.get("refresh_token_updater"): + if definition.get("refresh_token_updater") is not None: return True return any( ConcurrentDeclarativeSource._manifest_writes_back_config(value) diff --git a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml index 7506806b4..1de34175d 100644 --- a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml +++ b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml @@ -551,7 +551,7 @@ definitions: "$ref": "#/definitions/DynamicStreamCheckConfig" config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. type: object additionalProperties: true examples: @@ -597,7 +597,7 @@ definitions: default: true config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. type: object additionalProperties: true examples: diff --git a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py index 803e820c1..25b50bdd9 100644 --- a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py +++ b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py @@ -79,7 +79,7 @@ class CheckDynamicStream(BaseModel): ) config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) @@ -1789,7 +1789,7 @@ class CheckStream(BaseModel): dynamic_streams_check_configs: Optional[List[DynamicStreamCheckConfig]] = None config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index 5c46c1bdd..548fdf5d5 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -1201,16 +1201,31 @@ def test_given_config_overrides_on_check_dynamic_stream_then_components_see_them } -def _manifest_with_refresh_token_updater(check_component): +def _manifest_with_refresh_token_updater(check_component, refresh_token_updater=None): manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) manifest["check"] = check_component - manifest["streams"][0]["retriever"]["requester"]["authenticator"] = deepcopy( - _OAUTH_WITH_REFRESH_TOKEN_UPDATER - ) + authenticator = deepcopy(_OAUTH_WITH_REFRESH_TOKEN_UPDATER) + if refresh_token_updater is not None: + authenticator["refresh_token_updater"] = deepcopy(refresh_token_updater) + manifest["streams"][0]["retriever"]["requester"]["authenticator"] = authenticator return manifest -def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_rejected(): +@pytest.mark.parametrize( + "refresh_token_updater", + [ + pytest.param( + {"type": "RefreshTokenUpdater", "refresh_token_name": "refresh_token"}, id="populated" + ), + # Every field of `RefreshTokenUpdater` has a default, so an empty mapping is a valid way to take + # all of them. It builds the same single-use authenticator a populated one does, and the + # transformer injects no `type` into it, so it stays falsy - a truthiness test would miss it. + pytest.param({}, id="empty-taking-all-defaults"), + ], +) +def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_rejected( + refresh_token_updater, +): """A `refresh_token_updater` emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config. The restore cannot recall a message already on stdout, so the combination is refused.""" @@ -1220,7 +1235,8 @@ def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_reje "type": "CheckStream", "stream_names": ["items"], "config_overrides": {"resource": "check-only"}, - } + }, + refresh_token_updater=refresh_token_updater, ), config=_CONFIG_DRIVEN_PATH_CONFIG, catalog=None, @@ -1231,6 +1247,28 @@ def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_reje source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) +def test_given_no_refresh_token_updater_when_config_overrides_then_manifest_is_accepted(): + """The scan must not reject every OAuth manifest. The same authenticator without the updater writes + nothing back, so the overlay is allowed.""" + manifest = _manifest_with_refresh_token_updater( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + del manifest["streams"][0]["retriever"]["requester"]["authenticator"]["refresh_token_updater"] + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + def test_given_refresh_token_updater_without_config_overrides_then_nothing_is_rejected(): """The rejection is scoped to the feature. A manifest that does not use `config_overrides` keeps working with a `refresh_token_updater` exactly as before, even though the manifest scan detects it.""" From 2639ce4f7d846ed55a6e672418decca949451841 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Thu, 20 Aug 2026 14:01:02 +0300 Subject: [PATCH 05/10] fix(low-code): stop the config-persistence scan from matching config values The scan for `refresh_token_updater` walks the whole raw manifest rather than only recognised authenticators, and that coarseness is load-bearing: it runs before references are resolved, so an authenticator reached through a `$ref` is found only because the walk also visits `definitions`. The net was too wide in two places that hold config values rather than components. A connector whose spec declares a property named `refresh_token_updater` lost `config_overrides` entirely - every use refused, naming an authenticator feature the manifest never declared - and an override of a config field by that name was refused for the same reason. Neither key can contain an authenticator: `spec` exists only at the top level of a manifest, and `config_overrides` only on `CheckStream` and `CheckDynamicStream`, so skipping both subtrees removes the false positives without narrowing detection. Adds a regression test per false positive, plus one for a `refresh_token_updater` behind a `$ref`, so the property the coarse walk exists to provide is pinned against a future narrowing. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 16 +++- .../declarative/checks/test_check_stream.py | 87 +++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index bbb54e4b0..92886be3b 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -715,10 +715,23 @@ def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> "applied to every later sync. Remove `config_overrides`, or drop the `refresh_token_updater`." ) + # Subtrees of a manifest that hold data rather than components, so a `refresh_token_updater` key + # found inside one is a name collision and not an authenticator. Neither key is declared on any + # component that could contain an authenticator: `spec` exists only at the top level of a manifest, + # and `config_overrides` only on `CheckStream` and `CheckDynamicStream`. + _NON_COMPONENT_MANIFEST_KEYS = frozenset({"spec", "config_overrides"}) + @staticmethod def _manifest_writes_back_config(definition: Any) -> bool: """Whether any component in the manifest emits the connector config back to the platform. + The scan is deliberately coarse - it looks for the key anywhere rather than only under a + recognised authenticator - because it runs on the raw manifest, before references are resolved. + An authenticator reached through a `$ref` is only found because the walk also visits + `definitions`, where it lives under its own key rather than under a requester. The two subtrees + in `_NON_COMPONENT_MANIFEST_KEYS` are the exception: they hold config values, so a matching key + there means a connector whose spec happens to declare a field by that name, not a token refresh. + Tested with `is not None` rather than for truthiness, to match the factory. Every field of `RefreshTokenUpdater` has a default, so `refresh_token_updater: {}` is a valid way to take all of them - and it builds a single-use authenticator just like a populated one, because the factory's @@ -730,7 +743,8 @@ def _manifest_writes_back_config(definition: Any) -> bool: return True return any( ConcurrentDeclarativeSource._manifest_writes_back_config(value) - for value in definition.values() + for key, value in definition.items() + if key not in ConcurrentDeclarativeSource._NON_COMPONENT_MANIFEST_KEYS ) if isinstance(definition, list): return any( diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index 548fdf5d5..819e301c8 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -1269,6 +1269,93 @@ def test_given_no_refresh_token_updater_when_config_overrides_then_manifest_is_a assert source._manifest_writes_back_config(source._source_config) is False +def test_given_spec_property_named_refresh_token_updater_then_overrides_are_allowed(): + """The scan walks the raw manifest looking for the key anywhere, because an authenticator reached + through a `$ref` is only found under `definitions`. A connector whose spec happens to declare a + config field by that name must not be caught by that net - nothing in `spec` is a component.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "resource": {"type": "string"}, + "refresh_token_updater": {"type": "string"}, + }, + }, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_override_of_a_field_named_refresh_token_updater_then_it_is_allowed(): + """`config_overrides` holds config values, not components, so a key that collides with the + authenticator field name is just a config field and must not trip the guard.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only", "refresh_token_updater": "check-only"}, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + +def test_given_refresh_token_updater_behind_a_ref_then_manifest_is_rejected(): + """The coarse walk is what makes a `$ref`-ed authenticator detectable at all: the raw manifest holds + only the reference, and the authenticator itself sits under `definitions`. Narrowing the walk must + not lose that.""" + manifest = _manifest_with_refresh_token_updater( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + manifest["definitions"] = { + "authenticator": manifest["streams"][0]["retriever"]["requester"]["authenticator"] + } + manifest["streams"][0]["retriever"]["requester"]["authenticator"] = { + "$ref": "#/definitions/authenticator" + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with pytest.raises(ValueError, match="refresh_token_updater"): + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + + def test_given_refresh_token_updater_without_config_overrides_then_nothing_is_rejected(): """The rejection is scoped to the feature. A manifest that does not use `config_overrides` keeps working with a `refresh_token_updater` exactly as before, even though the manifest scan detects it.""" From ff7609567065c8358721afae7422f2b4b09b3bc2 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Thu, 20 Aug 2026 15:12:12 +0300 Subject: [PATCH 06/10] fix(low-code): make config_overrides values opaque and its errors readable Round 3 review feedback. Five changes, all on the check-time overlay. Override values are now genuinely used as-is. `ManifestReferenceResolver` treats any string starting with `#/` as a reference wherever it appears, so a config value shaped like a pointer was replaced by whatever it resolved to - and an unresolvable one raised out of `_pre_process_manifest`, taking `spec`, `discover` and `read` down with it over a field only `check` ever reads. The resolver now leaves the subtree alone. Both guards raise `AirbyteTracedException` with `FailureType.config_error` instead of a bare `ValueError`. `AirbyteEntrypoint.check` catches only the former, so the messages these guards exist to deliver were escaping `run()` and being re-wrapped as a generic system error, with no CONNECTION_STATUS emitted at all. `_manifest_writes_back_config` now also requires a string `type` on the mapping holding the key. The docstring justifying the previous coarse walk was wrong - the scan runs on `self._source_config`, which is post-`_pre_process_manifest`, so a `$ref`-ed authenticator is already inlined at the requester and carries its own type. Matching on component identity closes five false positives where a manifest declaring no authenticator at all was refused, and `_NON_COMPONENT_MANIFEST_KEYS` comes out with it. Override values that land on an `airbyte_secret` field are registered with the secret filter. The entrypoint builds that list from the config the user supplied, so a substituted value would print in the clear at a path where the user's own value prints as `****`. Two smaller ones: non-string keys are refused with an author-facing message rather than a `TypeError` from a join, and the undeclared-key warning now unions `allOf`/`anyOf`/`oneOf` branches so a composed spec does not get a warning about a field it declares. Tests cover each of the above. Seven mutations of the new production lines were checked and each is caught by exactly the test that names it, including deleting the schema blocks, which nothing pinned before. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 148 ++++++-- .../declarative_component_schema.yaml | 4 +- .../models/declarative_component_schema.py | 4 +- .../parsers/manifest_reference_resolver.py | 12 +- .../declarative/checks/test_check_stream.py | 338 +++++++++++++++++- 5 files changed, 463 insertions(+), 43 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index 92886be3b..e28758bf8 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -102,6 +102,7 @@ DebugSliceLogger, SliceLogger, ) +from airbyte_cdk.utils.airbyte_secrets_utils import add_to_secrets, get_secrets from airbyte_cdk.utils.stream_status_utils import as_airbyte_message from airbyte_cdk.utils.traced_exception import AirbyteTracedException @@ -659,9 +660,11 @@ def _config_overridden_for_check( yield return + self._raise_on_non_string_override_keys(config_overrides) self._raise_on_reserved_override_keys(config_overrides) self._raise_if_config_is_persisted(config_overrides) self._warn_on_unknown_override_keys(config_overrides) + self._register_override_secrets(config_overrides) # Keys only. An override may name a secret field, so values must not reach the logs. self.logger.info( f"Overriding config keys for the check operation: {', '.join(sorted(config_overrides))}" @@ -685,9 +688,34 @@ def _raise_on_reserved_override_keys(config_overrides: Mapping[str, Any]) -> Non """ reserved = sorted(key for key in config_overrides if key.startswith("__airbyte")) if reserved: - raise ValueError( - f"`config_overrides` may not set the reserved key(s) {reserved}. Keys prefixed with " - "`__airbyte` belong to the platform, not to the connector's spec." + raise AirbyteTracedException( + message=( + f"`config_overrides` may not set the reserved key(s) {reserved}. Keys prefixed " + "with `__airbyte` belong to the platform, not to the connector's spec." + ), + internal_message="config_overrides rejected: reserved __airbyte key", + failure_type=FailureType.config_error, + ) + + @staticmethod + def _raise_on_non_string_override_keys(config_overrides: Mapping[str, Any]) -> None: + """Reject non-string keys before anything downstream assumes `str`. + + The schema declares `type: object` with no `propertyNames` constraint, and YAML is happy to + produce `config_overrides: {0: 5}`. Every consumer from here on - the reserved-prefix test, the + undeclared-key warning, the log line - treats a key as a string, and a config field cannot be + addressed by a non-string name anyway, so this is a manifest error rather than something to + coerce. + """ + non_strings = [key for key in config_overrides if not isinstance(key, str)] + if non_strings: + raise AirbyteTracedException( + message=( + f"`config_overrides` keys must be strings, but {sorted(map(repr, non_strings))} " + "are not. Quote them in the manifest so they name a field in the connector's spec." + ), + internal_message="config_overrides rejected: non-string key", + failure_type=FailureType.config_error, ) def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> None: @@ -707,44 +735,55 @@ def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> """ if not self._manifest_writes_back_config(self._source_config): return - raise ValueError( - "`config_overrides` cannot be used by a manifest that declares a `refresh_token_updater`. " - "A token refresh during `check` emits the whole config it was handed as a CONNECTOR_CONFIG " - "control message, which the platform persists, so the check-only " - f"override(s) {sorted(config_overrides)} would be saved as this connection's config and " - "applied to every later sync. Remove `config_overrides`, or drop the `refresh_token_updater`." + raise AirbyteTracedException( + message=( + "`config_overrides` cannot be used by a manifest that declares a " + "`refresh_token_updater`. A token refresh during `check` emits the whole config it was " + "handed as a CONNECTOR_CONFIG control message, which the platform persists, so the " + f"check-only override(s) {sorted(config_overrides)} would be saved as this connection's " + "config and applied to every later sync. Remove `config_overrides`, or drop the " + "`refresh_token_updater`." + ), + internal_message="config_overrides rejected: manifest persists config via refresh_token_updater", + failure_type=FailureType.config_error, ) - # Subtrees of a manifest that hold data rather than components, so a `refresh_token_updater` key - # found inside one is a name collision and not an authenticator. Neither key is declared on any - # component that could contain an authenticator: `spec` exists only at the top level of a manifest, - # and `config_overrides` only on `CheckStream` and `CheckDynamicStream`. - _NON_COMPONENT_MANIFEST_KEYS = frozenset({"spec", "config_overrides"}) - @staticmethod def _manifest_writes_back_config(definition: Any) -> bool: """Whether any component in the manifest emits the connector config back to the platform. - The scan is deliberately coarse - it looks for the key anywhere rather than only under a - recognised authenticator - because it runs on the raw manifest, before references are resolved. - An authenticator reached through a `$ref` is only found because the walk also visits - `definitions`, where it lives under its own key rather than under a requester. The two subtrees - in `_NON_COMPONENT_MANIFEST_KEYS` are the exception: they hold config values, so a matching key - there means a connector whose spec happens to declare a field by that name, not a token refresh. - - Tested with `is not None` rather than for truthiness, to match the factory. Every field of - `RefreshTokenUpdater` has a default, so `refresh_token_updater: {}` is a valid way to take all of - them - and it builds a single-use authenticator just like a populated one, because the factory's - `if model.refresh_token_updater:` sees a model instance, which is always truthy. A truthiness test - here would see an empty dict and let that manifest through. + The walk visits the whole manifest rather than following a known path to the authenticator, + because an authenticator can sit under any requester, inside a `SelectiveAuthenticator`, or in a + `ConditionalStreams` branch. It runs on `self._source_config`, which is the output of + `_pre_process_manifest` - references are already resolved and `$parameters` already propagated, + so a `$ref`-ed authenticator is found inlined at the requester and carries its own `type`. + + Two conditions, and both are needed: + - `is not None` rather than truthiness, to match the factory. Every field of + `RefreshTokenUpdater` has a default, so `refresh_token_updater: {}` is a valid way to take + all of them - and it builds a single-use authenticator just like a populated one, because + the factory's `if model.refresh_token_updater:` sees a model instance, which is always + truthy. A truthiness test here would let that manifest through. + - a string `type` on the mapping that holds the key, which is what separates a component from + a data blob. `refresh_token_updater` is declared on `OAuthAuthenticator` and nowhere else, + the schema makes `type` required there, and the transformer injects one for a `class_name` + authenticator - so every real declaration has it. A record schema property, a + `request_body_json` entry or a `schemas` block that happens to use the same name does not, + and matching those would refuse a manifest that declares no authenticator at all. + + Known gap: a `CustomAuthenticator` whose `class_name` points at a class that emits a + CONNECTOR_CONFIG message is not detected, because nothing in the manifest names the behaviour. + Custom code is only permitted for a trusted manifest, so this is a documented limit rather than + an open hole. """ if isinstance(definition, Mapping): - if definition.get("refresh_token_updater") is not None: + if definition.get("refresh_token_updater") is not None and isinstance( + definition.get("type"), str + ): return True return any( ConcurrentDeclarativeSource._manifest_writes_back_config(value) - for key, value in definition.items() - if key not in ConcurrentDeclarativeSource._NON_COMPONENT_MANIFEST_KEYS + for value in definition.values() ) if isinstance(definition, list): return any( @@ -763,8 +802,8 @@ def _warn_on_unknown_override_keys(self, config_overrides: Mapping[str, Any]) -> """ if not self._spec_component: return - declared = self._spec_component.connection_specification.get("properties") - if not isinstance(declared, Mapping): + declared = self._declared_config_properties(self._spec_component.connection_specification) + if declared is None: return unknown = sorted(key for key in config_overrides if key not in declared) if unknown: @@ -773,6 +812,51 @@ def _warn_on_unknown_override_keys(self, config_overrides: Mapping[str, Any]) -> "they will have no effect on any component that reads the config by field name." ) + @staticmethod + def _declared_config_properties( + connection_specification: Mapping[str, Any], + ) -> Optional[Set[str]]: + """Every field name a spec declares, or `None` when the spec does not enumerate them. + + A spec is not always a flat `properties` object. `oneOf` credentials blocks and `allOf` + composition both put declarations one level down, and reading only the top level reports a + declared field as undeclared - which sends an author chasing a warning about a key that is + fine. `None` means "cannot tell", and the caller stays quiet rather than guessing. + """ + if not isinstance(connection_specification, Mapping): + return None + + names: Set[str] = set() + found_any = False + properties = connection_specification.get("properties") + if isinstance(properties, Mapping): + found_any = True + names.update(str(key) for key in properties) + for keyword in ("allOf", "anyOf", "oneOf"): + for branch in connection_specification.get(keyword) or []: + nested = ConcurrentDeclarativeSource._declared_config_properties(branch) + if nested is not None: + found_any = True + names.update(nested) + return names if found_any else None + + def _register_override_secrets(self, config_overrides: Mapping[str, Any]) -> None: + """Register override values that land on an `airbyte_secret` field, so they get redacted. + + The entrypoint builds the secret list once, from the config the user supplied + (`entrypoint.py`, `update_secrets(get_secrets(...))`), so a value this overlay substitutes at a + secret path is unknown to `filter_secrets` - and would print in the clear where the user's own + value at the same path prints as `****`. Registering is additive and over-redaction is harmless, + so this errs towards registering. + """ + if not self._spec_component: + return + for secret in get_secrets( + self._spec_component.connection_specification, dict(config_overrides) + ): + if secret is not None: + add_to_secrets(str(secret)) + @property def dynamic_streams(self) -> List[Dict[str, Any]]: return self._dynamic_stream_configs( diff --git a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml index 1de34175d..34fb86954 100644 --- a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml +++ b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml @@ -551,7 +551,7 @@ definitions: "$ref": "#/definitions/DynamicStreamCheckConfig" config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. type: object additionalProperties: true examples: @@ -597,7 +597,7 @@ definitions: default: true config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. type: object additionalProperties: true examples: diff --git a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py index 25b50bdd9..e939628e8 100644 --- a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py +++ b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py @@ -79,7 +79,7 @@ class CheckDynamicStream(BaseModel): ) config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) @@ -1789,7 +1789,7 @@ class CheckStream(BaseModel): dynamic_streams_check_configs: Optional[List[DynamicStreamCheckConfig]] = None config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) diff --git a/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py b/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py index 1c5ae0485..66fd35942 100644 --- a/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py +++ b/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py @@ -3,6 +3,7 @@ # import re +from copy import deepcopy from typing import Any, Dict, Mapping, Set, Tuple, Union from airbyte_cdk.sources.declarative.parsers.custom_exceptions import ( @@ -12,6 +13,13 @@ REF_TAG = "$ref" +# Manifest fields whose values are connector config, not components. A reference is a string that +# starts with `#/`, so without this a config value shaped like a pointer would be replaced by whatever +# it happens to resolve to - or, if it resolves to nothing, would raise out of `__init__` and take +# `spec`, `discover` and `read` down with it, none of which ever read the field. Their contract is that +# values are used as-is, so the resolver leaves the subtree alone. +_FIELDS_HOLDING_CONFIG_VALUES = frozenset({"config_overrides"}) + class ManifestReferenceResolver: """ @@ -109,7 +117,9 @@ def preprocess_manifest(self, manifest: Mapping[str, Any]) -> Dict[str, Any]: def _evaluate_node(self, node: Any, manifest: Mapping[str, Any], visited: Set[Any]) -> Any: if isinstance(node, dict): evaluated_dict = { - k: self._evaluate_node(v, manifest, visited) + k: deepcopy(v) + if k in _FIELDS_HOLDING_CONFIG_VALUES + else self._evaluate_node(v, manifest, visited) for k, v in node.items() if not self._is_ref_key(k) } diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index 819e301c8..fbc818792 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -4,22 +4,39 @@ import json import logging +import pkgutil from copy import deepcopy from typing import Any, Iterable, Mapping, Optional from unittest.mock import MagicMock import pytest import requests +import yaml from jsonschema.exceptions import ValidationError -from airbyte_cdk.models import Status +from airbyte_cdk.entrypoint import AirbyteEntrypoint +from airbyte_cdk.models import ( + AirbyteConnectionStatus, + AirbyteMessage, + ConnectorSpecification, + FailureType, + Status, + Type, +) from airbyte_cdk.sources.declarative.checks.check_stream import CheckStream from airbyte_cdk.sources.declarative.concurrent_declarative_source import ( ConcurrentDeclarativeSource, ) +from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( + CheckDynamicStream as CheckDynamicStreamModel, +) +from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( + CheckStream as CheckStreamModel, +) from airbyte_cdk.sources.streams.core import Stream from airbyte_cdk.sources.streams.http import HttpStream from airbyte_cdk.test.mock_http import HttpMocker, HttpRequest, HttpResponse +from airbyte_cdk.utils.traced_exception import AirbyteTracedException logger = logging.getLogger("test") config = dict() @@ -1243,8 +1260,9 @@ def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_reje state=None, ) - with pytest.raises(ValueError, match="refresh_token_updater"): + with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error def test_given_no_refresh_token_updater_when_config_overrides_then_manifest_is_accepted(): @@ -1268,6 +1286,11 @@ def test_given_no_refresh_token_updater_when_config_overrides_then_manifest_is_a assert source._manifest_writes_back_config(source._source_config) is False + # Asserted through the overlay rather than a full check: this manifest still authenticates with + # OAuth, and mocking a token exchange would test the handshake rather than the guard. + with source._config_overridden_for_check({"resource": "check-only"}): + assert source._config["resource"] == "check-only" + def test_given_spec_property_named_refresh_token_updater_then_overrides_are_allowed(): """The scan walks the raw manifest looking for the key anywhere, because an authenticator reached @@ -1326,6 +1349,12 @@ def test_given_override_of_a_field_named_refresh_token_updater_then_it_is_allowe assert source._manifest_writes_back_config(source._source_config) is False + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + def test_given_refresh_token_updater_behind_a_ref_then_manifest_is_rejected(): """The coarse walk is what makes a `$ref`-ed authenticator detectable at all: the raw manifest holds @@ -1352,8 +1381,9 @@ def test_given_refresh_token_updater_behind_a_ref_then_manifest_is_rejected(): state=None, ) - with pytest.raises(ValueError, match="refresh_token_updater"): + with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error def test_given_refresh_token_updater_without_config_overrides_then_nothing_is_rejected(): @@ -1393,7 +1423,7 @@ def test_given_config_overrides_when_check_then_overridden_keys_are_logged_witho ) with caplog.at_level(logging.INFO): - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED assert "Overriding config keys for the check operation: resource" in caplog.text assert "s3cr3t-value" not in caplog.text @@ -1428,7 +1458,7 @@ def test_given_override_key_absent_from_the_spec_then_a_warning_is_logged(caplog ) with caplog.at_level(logging.WARNING): - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED assert "typoed_key" in caplog.text assert "not declared in the connector spec" in caplog.text @@ -1447,5 +1477,301 @@ def test_given_an_airbyte_reserved_override_key_then_the_manifest_is_rejected(): } ) - with pytest.raises(ValueError, match="__airbyte_check_stream_names"): + with pytest.raises(AirbyteTracedException, match="__airbyte_check_stream_names") as raised: + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error + + +def _spec_with(properties): + return { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": properties, + }, + } + + +def test_given_a_reserved_key_when_check_through_the_entrypoint_then_a_failed_status_is_emitted(): + """The guards exist to hand a manifest author an actionable sentence. `AirbyteEntrypoint.check` + catches `AirbyteTracedException` and nothing else, so a bare `ValueError` would escape `run()`, be + re-wrapped as a generic system error, and emit no CONNECTION_STATUS at all - throwing away the very + message the guard was written to deliver.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"__airbyte_check_stream_names": ["something_else"]}, + } + ) + entrypoint = AirbyteEntrypoint(source) + + messages = list( + entrypoint.check( + ConnectorSpecification(connectionSpecification={}), _CONFIG_DRIVEN_PATH_CONFIG + ) + ) + + statuses = [ + message.connectionStatus for message in messages if message.type == Type.CONNECTION_STATUS + ] + assert len(statuses) == 1 + assert statuses[0].status == Status.FAILED + assert "__airbyte_check_stream_names" in statuses[0].message + + +def test_given_a_config_override_shaped_like_a_reference_then_it_stays_a_literal(): + """`ManifestReferenceResolver` treats any string starting with `#/` as a reference, wherever it + appears. Override values are connector config, so the resolver has to leave them alone - otherwise a + config value that happens to look like a pointer is silently replaced by whatever it resolves to.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["definitions"] = {"somewhere": {"a": 1}} + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "#/definitions/somewhere"}, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source.resolved_manifest["check"]["config_overrides"] == { + "resource": "#/definitions/somewhere" + } + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/#/definitions/somewhere") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_an_unresolvable_reference_shaped_override_then_the_source_still_constructs(): + """The worst version of the same bug: an unresolvable pointer raises inside `_pre_process_manifest`, + which runs in `__init__` - so `spec`, `discover` and `read` would all die over a field only `check` + ever reads.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "#/nothing/here"}, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert [stream.name for stream in source.streams(_CONFIG_DRIVEN_PATH_CONFIG)] == ["items"] + + +@pytest.mark.parametrize( + "mutate_manifest, description", + [ + pytest.param( + lambda manifest: manifest["streams"][0]["schema_loader"]["schema"]["properties"].update( + {"refresh_token_updater": {"type": "string"}} + ), + "record schema property", + id="inline-schema-property", + ), + pytest.param( + lambda manifest: manifest["streams"][0]["retriever"]["requester"].update( + {"request_parameters": {"refresh_token_updater": "x"}} + ), + "request parameter name", + id="request-parameter-name", + ), + pytest.param( + lambda manifest: manifest.update( + { + "schemas": { + "items": {"properties": {"refresh_token_updater": {"type": "string"}}} + } + } + ), + "top-level schemas block", + id="top-level-schemas", + ), + ], +) +def test_given_the_name_appears_in_a_data_blob_then_overrides_are_still_allowed( + mutate_manifest, description +): + """The scan looks for `refresh_token_updater` anywhere, because an authenticator can sit under any + requester. What keeps that from catching data is the `type` on the mapping holding the key: a + component always has one, a record schema property or a request body entry does not. Without that + condition these manifests - which declare no authenticator at all - are refused with an error + telling the author to remove something that does not exist.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + mutate_manifest(manifest) + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest( + url="https://api.test.com/check-only", + query_params=manifest["streams"][0]["retriever"]["requester"].get("request_parameters"), + ) + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_a_nested_override_then_the_object_is_replaced_rather_than_merged(): + """The one-level merge is a documented semantic, not an accident: replacing the object wholesale is + what keeps "remove this nested key during check" expressible. A recursive merge would leave every + sibling key in place, so this asserts a sibling is gone.""" + config = {"resource": "sync", "settings": {"mode": "sync", "sibling": "present"}} + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"settings": {"mode": "check-only"}}, + } + manifest["streams"][0]["retriever"]["requester"]["url"] = ( + "https://api.test.com/{{ config['settings'].get('sibling', 'dropped') }}" + ) + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=config, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + replaced_request = HttpRequest(url="https://api.test.com/dropped") + http_mocker.get(replaced_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, config).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(replaced_request, 1) + + +def test_given_non_string_override_keys_then_the_manifest_is_rejected_cleanly(): + """`type: object` in the schema does not constrain key types, and YAML will happily produce an + integer key. Every consumer downstream treats a key as a string, so this is refused with an + author-facing message rather than a `TypeError` from a join.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {0: "check-only"}, + } + ) + + with pytest.raises(AirbyteTracedException, match="must be strings") as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error + + +def test_given_a_spec_composed_with_all_of_then_no_spurious_warning_is_logged(caplog): + """A spec does not have to enumerate its fields at the top level. Reading only `properties` reports + an `allOf`-composed field as undeclared, which sends an author chasing a warning about a key that is + perfectly valid.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {}, + "allOf": [{"properties": {"resource": {"type": "string"}}}], + }, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + with caplog.at_level(logging.WARNING): + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert "not declared in the connector spec" not in caplog.text + + +def test_given_an_override_on_a_secret_field_then_the_value_is_registered_for_redaction(): + """The entrypoint builds the secret list from the config the user supplied, so a value substituted + here is unknown to `filter_secrets` - and would print in the clear at a path where the user's own + value prints as `****`.""" + from airbyte_cdk.utils.airbyte_secrets_utils import filter_secrets, update_secrets + + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only-secret"}, + } + manifest["spec"] = _spec_with({"resource": {"type": "string", "airbyte_secret": True}}) + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + update_secrets([]) + try: + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only-secret"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert filter_secrets("saw check-only-secret") == "saw ****" + finally: + update_secrets([]) + + +def test_config_overrides_is_published_on_both_check_components(): + """The schema is the contract the Connector Builder and the manifest server read, and the factory + deliberately ignores the field - so nothing at runtime notices if it disappears from the published + surface.""" + schema = yaml.safe_load( + pkgutil.get_data( + "airbyte_cdk.sources.declarative", "declarative_component_schema.yaml" + ).decode() + ) + + for component in ("CheckStream", "CheckDynamicStream"): + assert "config_overrides" in schema["definitions"][component]["properties"] + + assert CheckStreamModel( + type="CheckStream", stream_names=["items"], config_overrides={"a": 1} + ).config_overrides == {"a": 1} + assert CheckDynamicStreamModel( + type="CheckDynamicStream", stream_count=1, config_overrides={"a": 1} + ).config_overrides == {"a": 1} From 76646a5620c8c00e09169cd9898b8a4fafe98d64 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Thu, 20 Aug 2026 15:24:19 +0300 Subject: [PATCH 07/10] fix(low-code): match the authenticator type exactly in the persistence scan Requiring a string `type` on the mapping that holds `refresh_token_updater` left one false positive behind: an object-valued override such as `{"type": "settings", "refresh_token_updater": "value"}` carries both, so the guard refused a manifest with no authenticator at all. That shape became reachable when `_NON_COMPONENT_MANIFEST_KEYS` came out, since `config_overrides` is no longer skipped. Matching the type exactly closes it. `refresh_token_updater` is declared on `OAuthAuthenticator` alone, so that type would be enough on its own; `CustomAuthenticator` is included because the transformer injects it for a `class_name` component, and custom code that declares the field in the manifest is the one case of config-persisting custom code the manifest actually names. Every true positive still matches, including an authenticator nested in a `SelectiveAuthenticator` or reached through a `$ref`. Both halves are pinned: reverting to the `isinstance` test fails the new override case, and dropping `CustomAuthenticator` fails the new custom-authenticator case. That second test matches the guard's own sentence rather than the field name, because a failure to import the custom class also mentions `refresh_token_updater` - the error echoes the component definition. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 32 ++++++++++----- .../declarative/checks/test_check_stream.py | 40 +++++++++++++++++++ 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index e28758bf8..932e68662 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -748,6 +748,14 @@ def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> failure_type=FailureType.config_error, ) + # Component types that can be handed the connector config and write it back. `refresh_token_updater` + # is declared on `OAuthAuthenticator` alone in the schema, so that would be enough on its own - + # `CustomAuthenticator` is here because the transformer injects that type for a `class_name` + # component, and custom code declaring the field in the manifest is the shape most likely to persist. + _CONFIG_PERSISTING_AUTHENTICATOR_TYPES = frozenset( + {"OAuthAuthenticator", "CustomAuthenticator"} + ) + @staticmethod def _manifest_writes_back_config(definition: Any) -> bool: """Whether any component in the manifest emits the connector config back to the platform. @@ -764,21 +772,23 @@ def _manifest_writes_back_config(definition: Any) -> bool: all of them - and it builds a single-use authenticator just like a populated one, because the factory's `if model.refresh_token_updater:` sees a model instance, which is always truthy. A truthiness test here would let that manifest through. - - a string `type` on the mapping that holds the key, which is what separates a component from - a data blob. `refresh_token_updater` is declared on `OAuthAuthenticator` and nowhere else, - the schema makes `type` required there, and the transformer injects one for a `class_name` - authenticator - so every real declaration has it. A record schema property, a - `request_body_json` entry or a `schemas` block that happens to use the same name does not, - and matching those would refuse a manifest that declares no authenticator at all. - - Known gap: a `CustomAuthenticator` whose `class_name` points at a class that emits a - CONNECTOR_CONFIG message is not detected, because nothing in the manifest names the behaviour. + - an authenticator `type` on the mapping that holds the key, which is what separates a + component from a data blob. `refresh_token_updater` is declared on `OAuthAuthenticator` and + nowhere else, and the schema makes `type` required there, so no real declaration is missed. + A record schema property, a `request_parameters` entry, a `schemas` block or an + object-valued `config_overrides` that happens to use the same name does not carry that + type, and matching those would refuse a manifest that declares no authenticator at all. + + Known gap: a `CustomAuthenticator` that emits a CONNECTOR_CONFIG message without declaring a + `refresh_token_updater` is not detected, because nothing in the manifest names the behaviour. Custom code is only permitted for a trusted manifest, so this is a documented limit rather than an open hole. """ if isinstance(definition, Mapping): - if definition.get("refresh_token_updater") is not None and isinstance( - definition.get("type"), str + if ( + definition.get("refresh_token_updater") is not None + and definition.get("type") + in ConcurrentDeclarativeSource._CONFIG_PERSISTING_AUTHENTICATOR_TYPES ): return True return any( diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index fbc818792..3f6127317 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -1599,6 +1599,13 @@ def test_given_an_unresolvable_reference_shaped_override_then_the_source_still_c "top-level schemas block", id="top-level-schemas", ), + pytest.param( + lambda manifest: manifest["check"]["config_overrides"].update( + {"credentials": {"type": "settings", "refresh_token_updater": "a-value"}} + ), + "object-valued override carrying both keys", + id="override-value-with-type-and-key", + ), ], ) def test_given_the_name_appears_in_a_data_blob_then_overrides_are_still_allowed( @@ -1775,3 +1782,36 @@ def test_config_overrides_is_published_on_both_check_components(): assert CheckDynamicStreamModel( type="CheckDynamicStream", stream_count=1, config_overrides={"a": 1} ).config_overrides == {"a": 1} + + +def test_given_a_custom_authenticator_declaring_a_refresh_token_updater_then_it_is_rejected(): + """`refresh_token_updater` is declared on `OAuthAuthenticator` alone, so matching that type would be + enough for the schema as written. `CustomAuthenticator` is matched too because the transformer + injects that type for a `class_name` component, and custom code that declares the field is the shape + most likely to write the config back - the one case of custom code the manifest does name.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + manifest["streams"][0]["retriever"]["requester"]["authenticator"] = { + "type": "CustomAuthenticator", + "class_name": "unit_tests.sources.declarative.checks.test_check_stream.NotBuilt", + "refresh_token_updater": {}, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + # Matched on the guard's own sentence: a failure to import the custom class also mentions + # `refresh_token_updater`, because the message echoes the component definition. + with pytest.raises( + AirbyteTracedException, match="cannot be used by a manifest that declares" + ) as raised: + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error From 27ffcea1ed10d2f8a83407ebc790561d7f8380f3 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Thu, 20 Aug 2026 16:32:51 +0300 Subject: [PATCH 08/10] refactor(low-code): trim config_overrides docs and move its tests No behaviour change. Three review points on style. The docstrings had grown into a transcript of the review that produced them - rationale for decisions already made, arguments against alternatives nobody will propose again. That rots in a shared file. Trimmed to the semantics a caller needs, with the one open caveat pointed at its issue rather than restated: airbytehq/airbyte-internal-issues#16995. Docstring-to-logic ratio across the eight new methods is now 68:109, down from roughly 200:120. `_CONFIG_PERSISTING_AUTHENTICATOR_TYPES` sat between two methods; moved up beside `_LOWEST_SAFE_CONCURRENCY_LEVEL` with the other class-level constants. `_FIELDS_HOLDING_CONFIG_VALUES` stays in the resolver, since that is the module that consumes it. Moved the 30 tests out of `checks/test_check_stream.py`, which was the wrong home: the behaviour is in `ConcurrentDeclarativeSource` and the check factories deliberately ignore `model.config_overrides`. They live in `test_concurrent_declarative_source_config_overrides.py`, beside `test_concurrent_declarative_source.py` rather than inside it, because that module is already 6.1k lines and this would have pushed it past 7k. Re-ran four mutations after the move to confirm the tests still bind to the code and not to their old location: dropping `CustomAuthenticator` from the type set, reverting the type test to `isinstance`, removing the resolver exemption, and removing the secret registration each fail exactly the tests that name them. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 152 +-- .../parsers/manifest_reference_resolver.py | 8 +- .../declarative/checks/test_check_stream.py | 891 ----------------- ...ent_declarative_source_config_overrides.py | 925 ++++++++++++++++++ 4 files changed, 980 insertions(+), 996 deletions(-) create mode 100644 unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index 932e68662..1c88691f6 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -145,6 +145,16 @@ class ConcurrentDeclarativeSource(Source): # because it has hit the limit of futures but not partition reader is consuming them. _LOWEST_SAFE_CONCURRENCY_LEVEL = 2 + # Component types that hold the connector config and can write it back. `refresh_token_updater` is + # declared on `OAuthAuthenticator` alone; `CustomAuthenticator` is the type the transformer injects + # for a `class_name` component. + _CONFIG_PERSISTING_AUTHENTICATOR_TYPES = frozenset( + {"OAuthAuthenticator", "CustomAuthenticator"} + ) + + # Manifest fields whose values are connector config rather than components. + _FIELDS_HOLDING_CONFIG_VALUES = frozenset({"config_overrides"}) + def __init__( self, catalog: Optional[ConfiguredAirbyteCatalog] = None, @@ -625,36 +635,21 @@ def _config_overridden_for_check( ) -> Iterator[None]: """Overlay the check component's `config_overrides` onto the config for the duration of a check. - A check and a sync legitimately want different behaviour from the same manifest: a check is - interactive and should fail fast with an actionable message, while a sync can afford to wait out - a rate limit window. Before this existed, expressing that difference required a Python - `check_connection` override that built a second component tree from a modified config, which is - not available to a manifest-only connector. - - Overlaying here is enough to reach every component the checker builds, because `streams()` - interpolates from `self._config` and ignores its own `config` argument. - - Semantics, all deliberate: - - Values are applied verbatim. They are not interpolated, so a value containing `{{ }}` reaches - components as that literal string. - - The merge is one level deep. Overriding a key whose value is an object replaces that object - rather than merging into it, which keeps removing a nested key expressible. - - The overlay happens long after `_migrate_and_transform_config`, so an override is neither - normalised itself nor propagated to fields derived from it by a `ConfigTransformation`. - - `config_validations` run against `self._config_for_validation`, so an override cannot fail a - validation written for the config as supplied. - - Two consequences of the copy being shallow and of `self._config` being shared state: - - Rebinding `self._config` is not thread safe. Acceptable only because `check` is one command - per process. - - The copy is shallow, so every nested object is the *same* object as in the config the source - was constructed with. A component that writes into a nested path - `dpath.new(config, - ("credentials", "access_token"), ...)` - therefore writes through to that shared dict, and the - restore does not undo it, because the restore only rebinds the top level. Only a write to a - top-level key is discarded. - - The write-through above is why a manifest that declares a `refresh_token_updater` is rejected - outright rather than documented: see `_raise_if_config_is_persisted`. + Rebinding `self._config` reaches the whole component tree, because `streams()` interpolates from + it and ignores its own `config` argument. Not thread safe; `check` is one command per process. + + Semantics: + - values are applied verbatim, never interpolated; + - the merge is one level deep, so an object-valued override replaces rather than merges; + - it happens after `_migrate_and_transform_config`, so derived fields are not recomputed; + - `config_validations` run against `self._config_for_validation`, not the overlay. + + The copy is shallow, so a component writing into a nested path writes through to the config the + source was constructed with and the restore does not undo it. Only top-level writes are + discarded, which is why `_raise_if_config_is_persisted` refuses config-persisting manifests. + + Known limitation: `$parameters` declared on a check component still propagate into object-valued + overrides. See https://github.com/airbytehq/airbyte-internal-issues/issues/16995. """ if not config_overrides: yield @@ -679,12 +674,10 @@ def _config_overridden_for_check( @staticmethod def _raise_on_reserved_override_keys(config_overrides: Mapping[str, Any]) -> None: - """Refuse to overlay keys in the platform's reserved `__airbyte` namespace. + """Refuse keys in the platform's reserved `__airbyte` namespace. - These are not connector config, they are the platform's channel into it: `CheckStream` reads - `__airbyte_check_stream_names` out of the very config this overlay writes to, so an override there - would change which streams a check tests. A manifest wanting that has `stream_names` for it. The - prefix is refused wholesale so future internal keys are covered too. + `CheckStream` reads `__airbyte_check_stream_names` out of the config this overlay writes to, so + an override there would change which streams a check tests. The whole prefix is refused. """ reserved = sorted(key for key in config_overrides if key.startswith("__airbyte")) if reserved: @@ -699,14 +692,7 @@ def _raise_on_reserved_override_keys(config_overrides: Mapping[str, Any]) -> Non @staticmethod def _raise_on_non_string_override_keys(config_overrides: Mapping[str, Any]) -> None: - """Reject non-string keys before anything downstream assumes `str`. - - The schema declares `type: object` with no `propertyNames` constraint, and YAML is happy to - produce `config_overrides: {0: 5}`. Every consumer from here on - the reserved-prefix test, the - undeclared-key warning, the log line - treats a key as a string, and a config field cannot be - addressed by a non-string name anyway, so this is a manifest error rather than something to - coerce. - """ + """Reject non-string keys, which YAML allows and every consumer below assumes away.""" non_strings = [key for key in config_overrides if not isinstance(key, str)] if non_strings: raise AirbyteTracedException( @@ -721,17 +707,10 @@ def _raise_on_non_string_override_keys(config_overrides: Mapping[str, Any]) -> N def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> None: """Reject `config_overrides` on a manifest whose authenticator writes the config back. - A `refresh_token_updater` turns an `OAuthAuthenticator` into a - `DeclarativeSingleUseRefreshTokenOauth2Authenticator`, which is handed the config the component - tree was built with - during a check, the overlay. When it refreshes, `_emit_control_message` - prints that *entire* dict as a `CONNECTOR_CONFIG` control message, and the platform persists it. - So a check-only override would become the connection's saved config and apply to every later sync. - - Restoring `self._config` afterwards cannot help: the control message is already on stdout. And no - amount of threading the config through properly would help either, because the hazard is inherent - to handing an overridden config to something whose job is to write the config back. Until the - emitter is fixed to emit the config it was given plus only the token fields it owns, refusing the - combination is the honest answer. + A `refresh_token_updater` builds a `DeclarativeSingleUseRefreshTokenOauth2Authenticator`, whose + `_emit_control_message` emits the entire config it was handed as a `CONNECTOR_CONFIG` message. + During a check that is the overlay, and the platform persists it, so a check-only override would + become the connection's saved config. The restore cannot recall a message already on stdout. """ if not self._manifest_writes_back_config(self._source_config): return @@ -748,41 +727,20 @@ def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> failure_type=FailureType.config_error, ) - # Component types that can be handed the connector config and write it back. `refresh_token_updater` - # is declared on `OAuthAuthenticator` alone in the schema, so that would be enough on its own - - # `CustomAuthenticator` is here because the transformer injects that type for a `class_name` - # component, and custom code declaring the field in the manifest is the shape most likely to persist. - _CONFIG_PERSISTING_AUTHENTICATOR_TYPES = frozenset( - {"OAuthAuthenticator", "CustomAuthenticator"} - ) - @staticmethod def _manifest_writes_back_config(definition: Any) -> bool: """Whether any component in the manifest emits the connector config back to the platform. - The walk visits the whole manifest rather than following a known path to the authenticator, - because an authenticator can sit under any requester, inside a `SelectiveAuthenticator`, or in a - `ConditionalStreams` branch. It runs on `self._source_config`, which is the output of - `_pre_process_manifest` - references are already resolved and `$parameters` already propagated, - so a `$ref`-ed authenticator is found inlined at the requester and carries its own `type`. - - Two conditions, and both are needed: - - `is not None` rather than truthiness, to match the factory. Every field of - `RefreshTokenUpdater` has a default, so `refresh_token_updater: {}` is a valid way to take - all of them - and it builds a single-use authenticator just like a populated one, because - the factory's `if model.refresh_token_updater:` sees a model instance, which is always - truthy. A truthiness test here would let that manifest through. - - an authenticator `type` on the mapping that holds the key, which is what separates a - component from a data blob. `refresh_token_updater` is declared on `OAuthAuthenticator` and - nowhere else, and the schema makes `type` required there, so no real declaration is missed. - A record schema property, a `request_parameters` entry, a `schemas` block or an - object-valued `config_overrides` that happens to use the same name does not carry that - type, and matching those would refuse a manifest that declares no authenticator at all. - - Known gap: a `CustomAuthenticator` that emits a CONNECTOR_CONFIG message without declaring a - `refresh_token_updater` is not detected, because nothing in the manifest names the behaviour. - Custom code is only permitted for a trusted manifest, so this is a documented limit rather than - an open hole. + Walks the whole manifest, since an authenticator can sit under any requester, inside a + `SelectiveAuthenticator`, or in a `ConditionalStreams` branch. Callers pass `self._source_config`, + which is post-`_pre_process_manifest`, so a `$ref`-ed authenticator is already inlined and + carries its own `type`. + + `is not None` rather than truthiness because `refresh_token_updater: {}` takes every default and + still builds a single-use authenticator - the factory tests a model instance, which is truthy. + The type condition is what separates a component from a data blob that happens to use the name. + + Does not detect a `CustomAuthenticator` that persists config without declaring the field. """ if isinstance(definition, Mapping): if ( @@ -805,10 +763,8 @@ def _manifest_writes_back_config(definition: Any) -> bool: def _warn_on_unknown_override_keys(self, config_overrides: Mapping[str, Any]) -> None: """Warn about override keys the spec does not declare. - The overlay is the one part of the config nothing validates - the entrypoint validates what the - user supplied, and `config_validations` deliberately run against `self._config_for_validation`. So - a typo or a since-renamed field is a silent no-op that surfaces much later as "why does check no - longer fail fast". + Nothing validates the overlay - the entrypoint validates what the user supplied, and + `config_validations` run against `self._config_for_validation` - so a typo is a silent no-op. """ if not self._spec_component: return @@ -826,12 +782,10 @@ def _warn_on_unknown_override_keys(self, config_overrides: Mapping[str, Any]) -> def _declared_config_properties( connection_specification: Mapping[str, Any], ) -> Optional[Set[str]]: - """Every field name a spec declares, or `None` when the spec does not enumerate them. + """Every field name a spec declares, or `None` when it does not enumerate them. - A spec is not always a flat `properties` object. `oneOf` credentials blocks and `allOf` - composition both put declarations one level down, and reading only the top level reports a - declared field as undeclared - which sends an author chasing a warning about a key that is - fine. `None` means "cannot tell", and the caller stays quiet rather than guessing. + `oneOf` and `allOf` composition put declarations one level down, so reading only the top-level + `properties` would report a declared field as undeclared. """ if not isinstance(connection_specification, Mapping): return None @@ -851,13 +805,11 @@ def _declared_config_properties( return names if found_any else None def _register_override_secrets(self, config_overrides: Mapping[str, Any]) -> None: - """Register override values that land on an `airbyte_secret` field, so they get redacted. + """Register override values landing on an `airbyte_secret` field, so they get redacted. - The entrypoint builds the secret list once, from the config the user supplied - (`entrypoint.py`, `update_secrets(get_secrets(...))`), so a value this overlay substitutes at a - secret path is unknown to `filter_secrets` - and would print in the clear where the user's own - value at the same path prints as `****`. Registering is additive and over-redaction is harmless, - so this errs towards registering. + The entrypoint builds the secret list from the config the user supplied, so a value substituted + here is unknown to `filter_secrets`. Uses the same discovery as the entrypoint, so the overlay is + redacted exactly where the user's own value at that path would be. """ if not self._spec_component: return diff --git a/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py b/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py index 66fd35942..0294171d9 100644 --- a/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py +++ b/airbyte_cdk/sources/declarative/parsers/manifest_reference_resolver.py @@ -13,11 +13,9 @@ REF_TAG = "$ref" -# Manifest fields whose values are connector config, not components. A reference is a string that -# starts with `#/`, so without this a config value shaped like a pointer would be replaced by whatever -# it happens to resolve to - or, if it resolves to nothing, would raise out of `__init__` and take -# `spec`, `discover` and `read` down with it, none of which ever read the field. Their contract is that -# values are used as-is, so the resolver leaves the subtree alone. +# Manifest fields whose values are connector config rather than components. Any string starting with +# `#/` is a reference, so without this a config value shaped like a pointer would be resolved - or, if +# it resolves to nothing, would raise during preprocessing and break every command. _FIELDS_HOLDING_CONFIG_VALUES = frozenset({"config_overrides"}) diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index 3f6127317..6b97ca782 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -924,894 +924,3 @@ def test_check_stream_only_type_provided(): ) with pytest.raises(ValueError): source.check(logger, _CONFIG) - - -_CONFIG_DRIVEN_PATH_CONFIG = {"resource": "sync"} - -_MANIFEST_WITH_CONFIG_DRIVEN_PATH = { - "version": "6.7.0", - "type": "DeclarativeSource", - "check": {"type": "CheckStream", "stream_names": ["items"]}, - "streams": [ - { - "type": "DeclarativeStream", - "name": "items", - "primary_key": "id", - "schema_loader": { - "type": "InlineSchemaLoader", - "schema": { - "$schema": "http://json-schema.org/schema#", - "type": "object", - "properties": {"id": {"type": "integer"}}, - }, - }, - "retriever": { - "type": "SimpleRetriever", - "requester": { - "type": "HttpRequester", - "url": "https://api.test.com/{{ config['resource'] }}", - "http_method": "GET", - }, - "record_selector": { - "type": "RecordSelector", - "extractor": {"type": "DpathExtractor", "field_path": []}, - }, - "paginator": {"type": "NoPagination"}, - }, - } - ], -} - - -def _source_with_check_component(check_component): - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = check_component - return ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - -def test_given_no_config_overrides_when_check_then_components_use_the_user_config(): - source = _source_with_check_component({"type": "CheckStream", "stream_names": ["items"]}) - - with HttpMocker() as http_mocker: - # Only the user-configured path is mocked, so a request to any other path fails the test. - http_mocker.get( - HttpRequest(url="https://api.test.com/sync"), - HttpResponse(body=json.dumps([{"id": 1}])), - ) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - -def test_given_config_overrides_when_check_then_components_built_during_check_see_them(): - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - ) - - with HttpMocker() as http_mocker: - # Only the overridden path is mocked. Reaching this endpoint proves the overlay was applied to - # the stream the checker built, and not merely stored on the source. - overridden_request = HttpRequest(url="https://api.test.com/check-only") - http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - http_mocker.assert_number_of_calls(overridden_request, 1) - - -def test_given_config_overrides_when_check_then_the_config_is_restored_afterwards(): - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - ) - - with HttpMocker() as http_mocker: - http_mocker.get( - HttpRequest(url="https://api.test.com/check-only"), - HttpResponse(body=json.dumps([{"id": 1}])), - ) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - assert source._config == _CONFIG_DRIVEN_PATH_CONFIG - - # A sync that follows a check in the same process must go back to the user's value. - with HttpMocker() as http_mocker: - sync_request = HttpRequest(url="https://api.test.com/sync") - http_mocker.get(sync_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - stream = source.streams(_CONFIG_DRIVEN_PATH_CONFIG)[0] - assert stream.check_availability().is_available - - http_mocker.assert_number_of_calls(sync_request, 1) - - -def test_given_config_overrides_when_check_raises_then_the_config_is_restored(): - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["not_in_the_catalog"], - "config_overrides": {"resource": "check-only"}, - } - ) - - with pytest.raises(ValueError): - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - - assert source._config == _CONFIG_DRIVEN_PATH_CONFIG - - -def test_given_config_overrides_when_check_then_config_validations_run_against_the_user_config(): - """An override is authored in the manifest, so it must not be held to validations written for the - user's own input - the user has no way to satisfy them.""" - config = {"resource": "sync", "settings": {"mode": "sync"}} - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only", "settings": {"mode": "check-only"}}, - } - manifest["spec"] = { - "type": "Spec", - "connection_specification": { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": { - "resource": {"type": "string"}, - "settings": {"type": "object"}, - }, - }, - "config_normalization_rules": { - "type": "ConfigNormalizationRules", - "validations": [ - { - "type": "DpathValidator", - "field_path": ["settings"], - "validation_strategy": { - "type": "ValidateAdheresToSchema", - "base_schema": { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": {"mode": {"type": "string", "enum": ["sync"]}}, - "required": ["mode"], - "additionalProperties": False, - }, - }, - } - ], - }, - } - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=config, - catalog=None, - state=None, - ) - - with HttpMocker() as http_mocker: - http_mocker.get( - HttpRequest(url="https://api.test.com/check-only"), - HttpResponse(body=json.dumps([{"id": 1}])), - ) - - # `settings.mode` is overridden to a value outside the validator's enum, so this would fail were - # the overlay validated instead of the config the user supplied. - assert source.check(logger, config).status == Status.SUCCEEDED - - -def test_given_config_overrides_when_check_then_values_are_not_interpolated(): - """Pins the verbatim contract. A value containing `{{ }}` reaches components as that literal string, - so turning interpolation on later is a deliberate, test-breaking decision rather than a silent - reinterpretation of overrides already written.""" - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "{{config['resource']}}"}, - } - ) - - with HttpMocker() as http_mocker: - literal_request = HttpRequest(url="https://api.test.com/%7B%7Bconfig%5B'resource'%5D%7D%7D") - http_mocker.get(literal_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - http_mocker.assert_number_of_calls(literal_request, 1) - - -_MANIFEST_WITH_CONFIG_DRIVEN_DYNAMIC_STREAM = { - "version": "6.7.0", - "type": "DeclarativeSource", - "check": {"type": "CheckDynamicStream", "stream_count": 1}, - "streams": [], - "dynamic_streams": [ - { - "type": "DynamicDeclarativeStream", - "name": "dynamic_items", - "stream_template": { - "type": "DeclarativeStream", - "name": "", - "primary_key": [], - "schema_loader": { - "type": "InlineSchemaLoader", - "schema": { - "$schema": "http://json-schema.org/schema#", - "type": "object", - "properties": {"id": {"type": "integer"}}, - }, - }, - "retriever": { - "type": "SimpleRetriever", - "requester": { - "type": "HttpRequester", - "url": "https://api.test.com/{{ config['resource'] }}", - "http_method": "GET", - }, - "record_selector": { - "type": "RecordSelector", - "extractor": {"type": "DpathExtractor", "field_path": []}, - }, - "paginator": {"type": "NoPagination"}, - }, - }, - "components_resolver": { - "type": "ConfigComponentsResolver", - "stream_config": { - "type": "StreamConfig", - "configs_pointer": ["custom_streams"], - }, - "components_mapping": [ - { - "type": "ComponentMappingDefinition", - "field_path": ["name"], - "value": "{{components_values['name']}}", - } - ], - }, - } - ], -} - - -def test_given_config_overrides_on_check_dynamic_stream_then_components_see_them(): - """The overlay is read from the raw check definition, so it is checker-agnostic. Without this test a - refactor moving the read into `create_check_stream` would silently drop `CheckDynamicStream`.""" - config = {"resource": "sync", "custom_streams": [{"name": "items"}]} - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_DYNAMIC_STREAM) - manifest["check"] = { - "type": "CheckDynamicStream", - "stream_count": 1, - "config_overrides": {"resource": "check-only"}, - } - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=config, - catalog=None, - state=None, - ) - - with HttpMocker() as http_mocker: - overridden_request = HttpRequest(url="https://api.test.com/check-only") - http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, config).status == Status.SUCCEEDED - http_mocker.assert_number_of_calls(overridden_request, 1) - - -_OAUTH_WITH_REFRESH_TOKEN_UPDATER = { - "type": "OAuthAuthenticator", - "token_refresh_endpoint": "https://api.test.com/oauth/token", - "client_id": "{{ config['credentials']['client_id'] }}", - "client_secret": "{{ config['credentials']['client_secret'] }}", - "refresh_token": "{{ config['credentials']['refresh_token'] }}", - "refresh_token_updater": {"type": "RefreshTokenUpdater", "refresh_token_name": "refresh_token"}, -} - - -def _manifest_with_refresh_token_updater(check_component, refresh_token_updater=None): - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = check_component - authenticator = deepcopy(_OAUTH_WITH_REFRESH_TOKEN_UPDATER) - if refresh_token_updater is not None: - authenticator["refresh_token_updater"] = deepcopy(refresh_token_updater) - manifest["streams"][0]["retriever"]["requester"]["authenticator"] = authenticator - return manifest - - -@pytest.mark.parametrize( - "refresh_token_updater", - [ - pytest.param( - {"type": "RefreshTokenUpdater", "refresh_token_name": "refresh_token"}, id="populated" - ), - # Every field of `RefreshTokenUpdater` has a default, so an empty mapping is a valid way to take - # all of them. It builds the same single-use authenticator a populated one does, and the - # transformer injects no `type` into it, so it stays falsy - a truthiness test would miss it. - pytest.param({}, id="empty-taking-all-defaults"), - ], -) -def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_rejected( - refresh_token_updater, -): - """A `refresh_token_updater` emits the whole config it was handed as a CONNECTOR_CONFIG control - message, which the platform persists - so a check-only override would become the connection's saved - config. The restore cannot recall a message already on stdout, so the combination is refused.""" - source = ConcurrentDeclarativeSource( - source_config=_manifest_with_refresh_token_updater( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - }, - refresh_token_updater=refresh_token_updater, - ), - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error - - -def test_given_no_refresh_token_updater_when_config_overrides_then_manifest_is_accepted(): - """The scan must not reject every OAuth manifest. The same authenticator without the updater writes - nothing back, so the overlay is allowed.""" - manifest = _manifest_with_refresh_token_updater( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - ) - del manifest["streams"][0]["retriever"]["requester"]["authenticator"]["refresh_token_updater"] - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - assert source._manifest_writes_back_config(source._source_config) is False - - # Asserted through the overlay rather than a full check: this manifest still authenticates with - # OAuth, and mocking a token exchange would test the handshake rather than the guard. - with source._config_overridden_for_check({"resource": "check-only"}): - assert source._config["resource"] == "check-only" - - -def test_given_spec_property_named_refresh_token_updater_then_overrides_are_allowed(): - """The scan walks the raw manifest looking for the key anywhere, because an authenticator reached - through a `$ref` is only found under `definitions`. A connector whose spec happens to declare a - config field by that name must not be caught by that net - nothing in `spec` is a component.""" - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - manifest["spec"] = { - "type": "Spec", - "connection_specification": { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": { - "resource": {"type": "string"}, - "refresh_token_updater": {"type": "string"}, - }, - }, - } - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - assert source._manifest_writes_back_config(source._source_config) is False - - with HttpMocker() as http_mocker: - overridden_request = HttpRequest(url="https://api.test.com/check-only") - http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - -def test_given_override_of_a_field_named_refresh_token_updater_then_it_is_allowed(): - """`config_overrides` holds config values, not components, so a key that collides with the - authenticator field name is just a config field and must not trip the guard.""" - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only", "refresh_token_updater": "check-only"}, - } - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - assert source._manifest_writes_back_config(source._source_config) is False - - with HttpMocker() as http_mocker: - overridden_request = HttpRequest(url="https://api.test.com/check-only") - http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - -def test_given_refresh_token_updater_behind_a_ref_then_manifest_is_rejected(): - """The coarse walk is what makes a `$ref`-ed authenticator detectable at all: the raw manifest holds - only the reference, and the authenticator itself sits under `definitions`. Narrowing the walk must - not lose that.""" - manifest = _manifest_with_refresh_token_updater( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - ) - manifest["definitions"] = { - "authenticator": manifest["streams"][0]["retriever"]["requester"]["authenticator"] - } - manifest["streams"][0]["retriever"]["requester"]["authenticator"] = { - "$ref": "#/definitions/authenticator" - } - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error - - -def test_given_refresh_token_updater_without_config_overrides_then_nothing_is_rejected(): - """The rejection is scoped to the feature. A manifest that does not use `config_overrides` keeps - working with a `refresh_token_updater` exactly as before, even though the manifest scan detects it.""" - source = ConcurrentDeclarativeSource( - source_config=_manifest_with_refresh_token_updater( - {"type": "CheckStream", "stream_names": ["items"]} - ), - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - assert source._manifest_writes_back_config(source.resolved_manifest) is True - - # No overrides means no overlay, so the guard must not fire and the config must not be copied. - with source._config_overridden_for_check(None): - assert source._config is _CONFIG_DRIVEN_PATH_CONFIG - - -def test_given_config_overrides_when_check_then_overridden_keys_are_logged_without_values(caplog): - """An override may name a secret field, so the log records which keys were overridden but never what - they were set to.""" - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "s3cr3t-value"}, - } - ) - - with HttpMocker() as http_mocker: - http_mocker.get( - HttpRequest(url="https://api.test.com/s3cr3t-value"), - HttpResponse(body=json.dumps([{"id": 1}])), - ) - - with caplog.at_level(logging.INFO): - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - assert "Overriding config keys for the check operation: resource" in caplog.text - assert "s3cr3t-value" not in caplog.text - - -def test_given_override_key_absent_from_the_spec_then_a_warning_is_logged(caplog): - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only", "typoed_key": 1}, - } - manifest["spec"] = { - "type": "Spec", - "connection_specification": { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": {"resource": {"type": "string"}}, - }, - } - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - with HttpMocker() as http_mocker: - http_mocker.get( - HttpRequest(url="https://api.test.com/check-only"), - HttpResponse(body=json.dumps([{"id": 1}])), - ) - - with caplog.at_level(logging.WARNING): - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - assert "typoed_key" in caplog.text - assert "not declared in the connector spec" in caplog.text - assert "'resource'" not in caplog.text - - -def test_given_an_airbyte_reserved_override_key_then_the_manifest_is_rejected(): - """`__airbyte`-prefixed keys are the platform's channel into the config - `CheckStream` reads - `__airbyte_check_stream_names` from the very config the overlay writes to. The feature refuses to - touch that namespace rather than leaving it available.""" - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"__airbyte_check_stream_names": ["something_else"]}, - } - ) - - with pytest.raises(AirbyteTracedException, match="__airbyte_check_stream_names") as raised: - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error - - -def _spec_with(properties): - return { - "type": "Spec", - "connection_specification": { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": properties, - }, - } - - -def test_given_a_reserved_key_when_check_through_the_entrypoint_then_a_failed_status_is_emitted(): - """The guards exist to hand a manifest author an actionable sentence. `AirbyteEntrypoint.check` - catches `AirbyteTracedException` and nothing else, so a bare `ValueError` would escape `run()`, be - re-wrapped as a generic system error, and emit no CONNECTION_STATUS at all - throwing away the very - message the guard was written to deliver.""" - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"__airbyte_check_stream_names": ["something_else"]}, - } - ) - entrypoint = AirbyteEntrypoint(source) - - messages = list( - entrypoint.check( - ConnectorSpecification(connectionSpecification={}), _CONFIG_DRIVEN_PATH_CONFIG - ) - ) - - statuses = [ - message.connectionStatus for message in messages if message.type == Type.CONNECTION_STATUS - ] - assert len(statuses) == 1 - assert statuses[0].status == Status.FAILED - assert "__airbyte_check_stream_names" in statuses[0].message - - -def test_given_a_config_override_shaped_like_a_reference_then_it_stays_a_literal(): - """`ManifestReferenceResolver` treats any string starting with `#/` as a reference, wherever it - appears. Override values are connector config, so the resolver has to leave them alone - otherwise a - config value that happens to look like a pointer is silently replaced by whatever it resolves to.""" - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["definitions"] = {"somewhere": {"a": 1}} - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "#/definitions/somewhere"}, - } - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - assert source.resolved_manifest["check"]["config_overrides"] == { - "resource": "#/definitions/somewhere" - } - - with HttpMocker() as http_mocker: - overridden_request = HttpRequest(url="https://api.test.com/#/definitions/somewhere") - http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - -def test_given_an_unresolvable_reference_shaped_override_then_the_source_still_constructs(): - """The worst version of the same bug: an unresolvable pointer raises inside `_pre_process_manifest`, - which runs in `__init__` - so `spec`, `discover` and `read` would all die over a field only `check` - ever reads.""" - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "#/nothing/here"}, - } - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - assert [stream.name for stream in source.streams(_CONFIG_DRIVEN_PATH_CONFIG)] == ["items"] - - -@pytest.mark.parametrize( - "mutate_manifest, description", - [ - pytest.param( - lambda manifest: manifest["streams"][0]["schema_loader"]["schema"]["properties"].update( - {"refresh_token_updater": {"type": "string"}} - ), - "record schema property", - id="inline-schema-property", - ), - pytest.param( - lambda manifest: manifest["streams"][0]["retriever"]["requester"].update( - {"request_parameters": {"refresh_token_updater": "x"}} - ), - "request parameter name", - id="request-parameter-name", - ), - pytest.param( - lambda manifest: manifest.update( - { - "schemas": { - "items": {"properties": {"refresh_token_updater": {"type": "string"}}} - } - } - ), - "top-level schemas block", - id="top-level-schemas", - ), - pytest.param( - lambda manifest: manifest["check"]["config_overrides"].update( - {"credentials": {"type": "settings", "refresh_token_updater": "a-value"}} - ), - "object-valued override carrying both keys", - id="override-value-with-type-and-key", - ), - ], -) -def test_given_the_name_appears_in_a_data_blob_then_overrides_are_still_allowed( - mutate_manifest, description -): - """The scan looks for `refresh_token_updater` anywhere, because an authenticator can sit under any - requester. What keeps that from catching data is the `type` on the mapping holding the key: a - component always has one, a record schema property or a request body entry does not. Without that - condition these manifests - which declare no authenticator at all - are refused with an error - telling the author to remove something that does not exist.""" - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - mutate_manifest(manifest) - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - assert source._manifest_writes_back_config(source._source_config) is False - - with HttpMocker() as http_mocker: - overridden_request = HttpRequest( - url="https://api.test.com/check-only", - query_params=manifest["streams"][0]["retriever"]["requester"].get("request_parameters"), - ) - http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - -def test_given_a_nested_override_then_the_object_is_replaced_rather_than_merged(): - """The one-level merge is a documented semantic, not an accident: replacing the object wholesale is - what keeps "remove this nested key during check" expressible. A recursive merge would leave every - sibling key in place, so this asserts a sibling is gone.""" - config = {"resource": "sync", "settings": {"mode": "sync", "sibling": "present"}} - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"settings": {"mode": "check-only"}}, - } - manifest["streams"][0]["retriever"]["requester"]["url"] = ( - "https://api.test.com/{{ config['settings'].get('sibling', 'dropped') }}" - ) - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=config, - catalog=None, - state=None, - ) - - with HttpMocker() as http_mocker: - replaced_request = HttpRequest(url="https://api.test.com/dropped") - http_mocker.get(replaced_request, HttpResponse(body=json.dumps([{"id": 1}]))) - - assert source.check(logger, config).status == Status.SUCCEEDED - http_mocker.assert_number_of_calls(replaced_request, 1) - - -def test_given_non_string_override_keys_then_the_manifest_is_rejected_cleanly(): - """`type: object` in the schema does not constrain key types, and YAML will happily produce an - integer key. Every consumer downstream treats a key as a string, so this is refused with an - author-facing message rather than a `TypeError` from a join.""" - source = _source_with_check_component( - { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {0: "check-only"}, - } - ) - - with pytest.raises(AirbyteTracedException, match="must be strings") as raised: - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error - - -def test_given_a_spec_composed_with_all_of_then_no_spurious_warning_is_logged(caplog): - """A spec does not have to enumerate its fields at the top level. Reading only `properties` reports - an `allOf`-composed field as undeclared, which sends an author chasing a warning about a key that is - perfectly valid.""" - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - manifest["spec"] = { - "type": "Spec", - "connection_specification": { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": {}, - "allOf": [{"properties": {"resource": {"type": "string"}}}], - }, - } - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - with HttpMocker() as http_mocker: - http_mocker.get( - HttpRequest(url="https://api.test.com/check-only"), - HttpResponse(body=json.dumps([{"id": 1}])), - ) - - with caplog.at_level(logging.WARNING): - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - assert "not declared in the connector spec" not in caplog.text - - -def test_given_an_override_on_a_secret_field_then_the_value_is_registered_for_redaction(): - """The entrypoint builds the secret list from the config the user supplied, so a value substituted - here is unknown to `filter_secrets` - and would print in the clear at a path where the user's own - value prints as `****`.""" - from airbyte_cdk.utils.airbyte_secrets_utils import filter_secrets, update_secrets - - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only-secret"}, - } - manifest["spec"] = _spec_with({"resource": {"type": "string", "airbyte_secret": True}}) - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - update_secrets([]) - try: - with HttpMocker() as http_mocker: - http_mocker.get( - HttpRequest(url="https://api.test.com/check-only-secret"), - HttpResponse(body=json.dumps([{"id": 1}])), - ) - - assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED - - assert filter_secrets("saw check-only-secret") == "saw ****" - finally: - update_secrets([]) - - -def test_config_overrides_is_published_on_both_check_components(): - """The schema is the contract the Connector Builder and the manifest server read, and the factory - deliberately ignores the field - so nothing at runtime notices if it disappears from the published - surface.""" - schema = yaml.safe_load( - pkgutil.get_data( - "airbyte_cdk.sources.declarative", "declarative_component_schema.yaml" - ).decode() - ) - - for component in ("CheckStream", "CheckDynamicStream"): - assert "config_overrides" in schema["definitions"][component]["properties"] - - assert CheckStreamModel( - type="CheckStream", stream_names=["items"], config_overrides={"a": 1} - ).config_overrides == {"a": 1} - assert CheckDynamicStreamModel( - type="CheckDynamicStream", stream_count=1, config_overrides={"a": 1} - ).config_overrides == {"a": 1} - - -def test_given_a_custom_authenticator_declaring_a_refresh_token_updater_then_it_is_rejected(): - """`refresh_token_updater` is declared on `OAuthAuthenticator` alone, so matching that type would be - enough for the schema as written. `CustomAuthenticator` is matched too because the transformer - injects that type for a `class_name` component, and custom code that declares the field is the shape - most likely to write the config back - the one case of custom code the manifest does name.""" - manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) - manifest["check"] = { - "type": "CheckStream", - "stream_names": ["items"], - "config_overrides": {"resource": "check-only"}, - } - manifest["streams"][0]["retriever"]["requester"]["authenticator"] = { - "type": "CustomAuthenticator", - "class_name": "unit_tests.sources.declarative.checks.test_check_stream.NotBuilt", - "refresh_token_updater": {}, - } - - source = ConcurrentDeclarativeSource( - source_config=manifest, - config=_CONFIG_DRIVEN_PATH_CONFIG, - catalog=None, - state=None, - ) - - # Matched on the guard's own sentence: a failure to import the custom class also mentions - # `refresh_token_updater`, because the message echoes the component definition. - with pytest.raises( - AirbyteTracedException, match="cannot be used by a manifest that declares" - ) as raised: - source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error diff --git a/unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py b/unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py new file mode 100644 index 000000000..dca9b76b6 --- /dev/null +++ b/unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py @@ -0,0 +1,925 @@ +# +# Copyright (c) 2025 Airbyte, Inc., all rights reserved. +# + +"""Tests for `ConcurrentDeclarativeSource._config_overridden_for_check` and its guards. + +The behaviour under test lives in `ConcurrentDeclarativeSource`, not in the check components - the +check factories deliberately ignore `model.config_overrides`. Kept beside +`test_concurrent_declarative_source.py` rather than inside it because that module is already 6k lines. +""" + +import json +import logging +import pkgutil +from copy import deepcopy + +import pytest +import yaml + +from airbyte_cdk.entrypoint import AirbyteEntrypoint +from airbyte_cdk.models import ConnectorSpecification, FailureType, Status, Type +from airbyte_cdk.sources.declarative.concurrent_declarative_source import ( + ConcurrentDeclarativeSource, +) +from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( + CheckDynamicStream as CheckDynamicStreamModel, +) +from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( + CheckStream as CheckStreamModel, +) +from airbyte_cdk.test.mock_http import HttpMocker, HttpRequest, HttpResponse +from airbyte_cdk.utils.traced_exception import AirbyteTracedException + +logger = logging.getLogger("test") + + +_CONFIG_DRIVEN_PATH_CONFIG = {"resource": "sync"} + +_MANIFEST_WITH_CONFIG_DRIVEN_PATH = { + "version": "6.7.0", + "type": "DeclarativeSource", + "check": {"type": "CheckStream", "stream_names": ["items"]}, + "streams": [ + { + "type": "DeclarativeStream", + "name": "items", + "primary_key": "id", + "schema_loader": { + "type": "InlineSchemaLoader", + "schema": { + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": {"id": {"type": "integer"}}, + }, + }, + "retriever": { + "type": "SimpleRetriever", + "requester": { + "type": "HttpRequester", + "url": "https://api.test.com/{{ config['resource'] }}", + "http_method": "GET", + }, + "record_selector": { + "type": "RecordSelector", + "extractor": {"type": "DpathExtractor", "field_path": []}, + }, + "paginator": {"type": "NoPagination"}, + }, + } + ], +} + + +def _source_with_check_component(check_component): + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = check_component + return ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + +def test_given_no_config_overrides_when_check_then_components_use_the_user_config(): + source = _source_with_check_component({"type": "CheckStream", "stream_names": ["items"]}) + + with HttpMocker() as http_mocker: + # Only the user-configured path is mocked, so a request to any other path fails the test. + http_mocker.get( + HttpRequest(url="https://api.test.com/sync"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_config_overrides_when_check_then_components_built_during_check_see_them(): + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + + with HttpMocker() as http_mocker: + # Only the overridden path is mocked. Reaching this endpoint proves the overlay was applied to + # the stream the checker built, and not merely stored on the source. + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(overridden_request, 1) + + +def test_given_config_overrides_when_check_then_the_config_is_restored_afterwards(): + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert source._config == _CONFIG_DRIVEN_PATH_CONFIG + + # A sync that follows a check in the same process must go back to the user's value. + with HttpMocker() as http_mocker: + sync_request = HttpRequest(url="https://api.test.com/sync") + http_mocker.get(sync_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + stream = source.streams(_CONFIG_DRIVEN_PATH_CONFIG)[0] + assert stream.check_availability().is_available + + http_mocker.assert_number_of_calls(sync_request, 1) + + +def test_given_config_overrides_when_check_raises_then_the_config_is_restored(): + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["not_in_the_catalog"], + "config_overrides": {"resource": "check-only"}, + } + ) + + with pytest.raises(ValueError): + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + + assert source._config == _CONFIG_DRIVEN_PATH_CONFIG + + +def test_given_config_overrides_when_check_then_config_validations_run_against_the_user_config(): + """An override is authored in the manifest, so it must not be held to validations written for the + user's own input - the user has no way to satisfy them.""" + config = {"resource": "sync", "settings": {"mode": "sync"}} + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only", "settings": {"mode": "check-only"}}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "resource": {"type": "string"}, + "settings": {"type": "object"}, + }, + }, + "config_normalization_rules": { + "type": "ConfigNormalizationRules", + "validations": [ + { + "type": "DpathValidator", + "field_path": ["settings"], + "validation_strategy": { + "type": "ValidateAdheresToSchema", + "base_schema": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {"mode": {"type": "string", "enum": ["sync"]}}, + "required": ["mode"], + "additionalProperties": False, + }, + }, + } + ], + }, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=config, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + # `settings.mode` is overridden to a value outside the validator's enum, so this would fail were + # the overlay validated instead of the config the user supplied. + assert source.check(logger, config).status == Status.SUCCEEDED + + +def test_given_config_overrides_when_check_then_values_are_not_interpolated(): + """Pins the verbatim contract. A value containing `{{ }}` reaches components as that literal string, + so turning interpolation on later is a deliberate, test-breaking decision rather than a silent + reinterpretation of overrides already written.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "{{config['resource']}}"}, + } + ) + + with HttpMocker() as http_mocker: + literal_request = HttpRequest(url="https://api.test.com/%7B%7Bconfig%5B'resource'%5D%7D%7D") + http_mocker.get(literal_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(literal_request, 1) + + +_MANIFEST_WITH_CONFIG_DRIVEN_DYNAMIC_STREAM = { + "version": "6.7.0", + "type": "DeclarativeSource", + "check": {"type": "CheckDynamicStream", "stream_count": 1}, + "streams": [], + "dynamic_streams": [ + { + "type": "DynamicDeclarativeStream", + "name": "dynamic_items", + "stream_template": { + "type": "DeclarativeStream", + "name": "", + "primary_key": [], + "schema_loader": { + "type": "InlineSchemaLoader", + "schema": { + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": {"id": {"type": "integer"}}, + }, + }, + "retriever": { + "type": "SimpleRetriever", + "requester": { + "type": "HttpRequester", + "url": "https://api.test.com/{{ config['resource'] }}", + "http_method": "GET", + }, + "record_selector": { + "type": "RecordSelector", + "extractor": {"type": "DpathExtractor", "field_path": []}, + }, + "paginator": {"type": "NoPagination"}, + }, + }, + "components_resolver": { + "type": "ConfigComponentsResolver", + "stream_config": { + "type": "StreamConfig", + "configs_pointer": ["custom_streams"], + }, + "components_mapping": [ + { + "type": "ComponentMappingDefinition", + "field_path": ["name"], + "value": "{{components_values['name']}}", + } + ], + }, + } + ], +} + + +def test_given_config_overrides_on_check_dynamic_stream_then_components_see_them(): + """The overlay is read from the raw check definition, so it is checker-agnostic. Without this test a + refactor moving the read into `create_check_stream` would silently drop `CheckDynamicStream`.""" + config = {"resource": "sync", "custom_streams": [{"name": "items"}]} + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_DYNAMIC_STREAM) + manifest["check"] = { + "type": "CheckDynamicStream", + "stream_count": 1, + "config_overrides": {"resource": "check-only"}, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=config, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, config).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(overridden_request, 1) + + +_OAUTH_WITH_REFRESH_TOKEN_UPDATER = { + "type": "OAuthAuthenticator", + "token_refresh_endpoint": "https://api.test.com/oauth/token", + "client_id": "{{ config['credentials']['client_id'] }}", + "client_secret": "{{ config['credentials']['client_secret'] }}", + "refresh_token": "{{ config['credentials']['refresh_token'] }}", + "refresh_token_updater": {"type": "RefreshTokenUpdater", "refresh_token_name": "refresh_token"}, +} + + +def _manifest_with_refresh_token_updater(check_component, refresh_token_updater=None): + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = check_component + authenticator = deepcopy(_OAUTH_WITH_REFRESH_TOKEN_UPDATER) + if refresh_token_updater is not None: + authenticator["refresh_token_updater"] = deepcopy(refresh_token_updater) + manifest["streams"][0]["retriever"]["requester"]["authenticator"] = authenticator + return manifest + + +@pytest.mark.parametrize( + "refresh_token_updater", + [ + pytest.param( + {"type": "RefreshTokenUpdater", "refresh_token_name": "refresh_token"}, id="populated" + ), + # Every field of `RefreshTokenUpdater` has a default, so an empty mapping is a valid way to take + # all of them. It builds the same single-use authenticator a populated one does, and the + # transformer injects no `type` into it, so it stays falsy - a truthiness test would miss it. + pytest.param({}, id="empty-taking-all-defaults"), + ], +) +def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_rejected( + refresh_token_updater, +): + """A `refresh_token_updater` emits the whole config it was handed as a CONNECTOR_CONFIG control + message, which the platform persists - so a check-only override would become the connection's saved + config. The restore cannot recall a message already on stdout, so the combination is refused.""" + source = ConcurrentDeclarativeSource( + source_config=_manifest_with_refresh_token_updater( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + }, + refresh_token_updater=refresh_token_updater, + ), + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error + + +def test_given_no_refresh_token_updater_when_config_overrides_then_manifest_is_accepted(): + """The scan must not reject every OAuth manifest. The same authenticator without the updater writes + nothing back, so the overlay is allowed.""" + manifest = _manifest_with_refresh_token_updater( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + del manifest["streams"][0]["retriever"]["requester"]["authenticator"]["refresh_token_updater"] + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + # Asserted through the overlay rather than a full check: this manifest still authenticates with + # OAuth, and mocking a token exchange would test the handshake rather than the guard. + with source._config_overridden_for_check({"resource": "check-only"}): + assert source._config["resource"] == "check-only" + + +def test_given_spec_property_named_refresh_token_updater_then_overrides_are_allowed(): + """The scan walks the raw manifest looking for the key anywhere, because an authenticator reached + through a `$ref` is only found under `definitions`. A connector whose spec happens to declare a + config field by that name must not be caught by that net - nothing in `spec` is a component.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "resource": {"type": "string"}, + "refresh_token_updater": {"type": "string"}, + }, + }, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_override_of_a_field_named_refresh_token_updater_then_it_is_allowed(): + """`config_overrides` holds config values, not components, so a key that collides with the + authenticator field name is just a config field and must not trip the guard.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only", "refresh_token_updater": "check-only"}, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/check-only") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_refresh_token_updater_behind_a_ref_then_manifest_is_rejected(): + """The coarse walk is what makes a `$ref`-ed authenticator detectable at all: the raw manifest holds + only the reference, and the authenticator itself sits under `definitions`. Narrowing the walk must + not lose that.""" + manifest = _manifest_with_refresh_token_updater( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + ) + manifest["definitions"] = { + "authenticator": manifest["streams"][0]["retriever"]["requester"]["authenticator"] + } + manifest["streams"][0]["retriever"]["requester"]["authenticator"] = { + "$ref": "#/definitions/authenticator" + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error + + +def test_given_refresh_token_updater_without_config_overrides_then_nothing_is_rejected(): + """The rejection is scoped to the feature. A manifest that does not use `config_overrides` keeps + working with a `refresh_token_updater` exactly as before, even though the manifest scan detects it.""" + source = ConcurrentDeclarativeSource( + source_config=_manifest_with_refresh_token_updater( + {"type": "CheckStream", "stream_names": ["items"]} + ), + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source.resolved_manifest) is True + + # No overrides means no overlay, so the guard must not fire and the config must not be copied. + with source._config_overridden_for_check(None): + assert source._config is _CONFIG_DRIVEN_PATH_CONFIG + + +def test_given_config_overrides_when_check_then_overridden_keys_are_logged_without_values(caplog): + """An override may name a secret field, so the log records which keys were overridden but never what + they were set to.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "s3cr3t-value"}, + } + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/s3cr3t-value"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + with caplog.at_level(logging.INFO): + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert "Overriding config keys for the check operation: resource" in caplog.text + assert "s3cr3t-value" not in caplog.text + + +def test_given_override_key_absent_from_the_spec_then_a_warning_is_logged(caplog): + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only", "typoed_key": 1}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {"resource": {"type": "string"}}, + }, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + with caplog.at_level(logging.WARNING): + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert "typoed_key" in caplog.text + assert "not declared in the connector spec" in caplog.text + assert "'resource'" not in caplog.text + + +def test_given_an_airbyte_reserved_override_key_then_the_manifest_is_rejected(): + """`__airbyte`-prefixed keys are the platform's channel into the config - `CheckStream` reads + `__airbyte_check_stream_names` from the very config the overlay writes to. The feature refuses to + touch that namespace rather than leaving it available.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"__airbyte_check_stream_names": ["something_else"]}, + } + ) + + with pytest.raises(AirbyteTracedException, match="__airbyte_check_stream_names") as raised: + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error + + +def _spec_with(properties): + return { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": properties, + }, + } + + +def test_given_a_reserved_key_when_check_through_the_entrypoint_then_a_failed_status_is_emitted(): + """The guards exist to hand a manifest author an actionable sentence. `AirbyteEntrypoint.check` + catches `AirbyteTracedException` and nothing else, so a bare `ValueError` would escape `run()`, be + re-wrapped as a generic system error, and emit no CONNECTION_STATUS at all - throwing away the very + message the guard was written to deliver.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"__airbyte_check_stream_names": ["something_else"]}, + } + ) + entrypoint = AirbyteEntrypoint(source) + + messages = list( + entrypoint.check( + ConnectorSpecification(connectionSpecification={}), _CONFIG_DRIVEN_PATH_CONFIG + ) + ) + + statuses = [ + message.connectionStatus for message in messages if message.type == Type.CONNECTION_STATUS + ] + assert len(statuses) == 1 + assert statuses[0].status == Status.FAILED + assert "__airbyte_check_stream_names" in statuses[0].message + + +def test_given_a_config_override_shaped_like_a_reference_then_it_stays_a_literal(): + """`ManifestReferenceResolver` treats any string starting with `#/` as a reference, wherever it + appears. Override values are connector config, so the resolver has to leave them alone - otherwise a + config value that happens to look like a pointer is silently replaced by whatever it resolves to.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["definitions"] = {"somewhere": {"a": 1}} + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "#/definitions/somewhere"}, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source.resolved_manifest["check"]["config_overrides"] == { + "resource": "#/definitions/somewhere" + } + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest(url="https://api.test.com/#/definitions/somewhere") + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_an_unresolvable_reference_shaped_override_then_the_source_still_constructs(): + """The worst version of the same bug: an unresolvable pointer raises inside `_pre_process_manifest`, + which runs in `__init__` - so `spec`, `discover` and `read` would all die over a field only `check` + ever reads.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "#/nothing/here"}, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert [stream.name for stream in source.streams(_CONFIG_DRIVEN_PATH_CONFIG)] == ["items"] + + +@pytest.mark.parametrize( + "mutate_manifest, description", + [ + pytest.param( + lambda manifest: manifest["streams"][0]["schema_loader"]["schema"]["properties"].update( + {"refresh_token_updater": {"type": "string"}} + ), + "record schema property", + id="inline-schema-property", + ), + pytest.param( + lambda manifest: manifest["streams"][0]["retriever"]["requester"].update( + {"request_parameters": {"refresh_token_updater": "x"}} + ), + "request parameter name", + id="request-parameter-name", + ), + pytest.param( + lambda manifest: manifest.update( + { + "schemas": { + "items": {"properties": {"refresh_token_updater": {"type": "string"}}} + } + } + ), + "top-level schemas block", + id="top-level-schemas", + ), + pytest.param( + lambda manifest: manifest["check"]["config_overrides"].update( + {"credentials": {"type": "settings", "refresh_token_updater": "a-value"}} + ), + "object-valued override carrying both keys", + id="override-value-with-type-and-key", + ), + ], +) +def test_given_the_name_appears_in_a_data_blob_then_overrides_are_still_allowed( + mutate_manifest, description +): + """The scan looks for `refresh_token_updater` anywhere, because an authenticator can sit under any + requester. What keeps that from catching data is the `type` on the mapping holding the key: a + component always has one, a record schema property or a request body entry does not. Without that + condition these manifests - which declare no authenticator at all - are refused with an error + telling the author to remove something that does not exist.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + mutate_manifest(manifest) + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + assert source._manifest_writes_back_config(source._source_config) is False + + with HttpMocker() as http_mocker: + overridden_request = HttpRequest( + url="https://api.test.com/check-only", + query_params=manifest["streams"][0]["retriever"]["requester"].get("request_parameters"), + ) + http_mocker.get(overridden_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + +def test_given_a_nested_override_then_the_object_is_replaced_rather_than_merged(): + """The one-level merge is a documented semantic, not an accident: replacing the object wholesale is + what keeps "remove this nested key during check" expressible. A recursive merge would leave every + sibling key in place, so this asserts a sibling is gone.""" + config = {"resource": "sync", "settings": {"mode": "sync", "sibling": "present"}} + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"settings": {"mode": "check-only"}}, + } + manifest["streams"][0]["retriever"]["requester"]["url"] = ( + "https://api.test.com/{{ config['settings'].get('sibling', 'dropped') }}" + ) + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=config, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + replaced_request = HttpRequest(url="https://api.test.com/dropped") + http_mocker.get(replaced_request, HttpResponse(body=json.dumps([{"id": 1}]))) + + assert source.check(logger, config).status == Status.SUCCEEDED + http_mocker.assert_number_of_calls(replaced_request, 1) + + +def test_given_non_string_override_keys_then_the_manifest_is_rejected_cleanly(): + """`type: object` in the schema does not constrain key types, and YAML will happily produce an + integer key. Every consumer downstream treats a key as a string, so this is refused with an + author-facing message rather than a `TypeError` from a join.""" + source = _source_with_check_component( + { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {0: "check-only"}, + } + ) + + with pytest.raises(AirbyteTracedException, match="must be strings") as raised: + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error + + +def test_given_a_spec_composed_with_all_of_then_no_spurious_warning_is_logged(caplog): + """A spec does not have to enumerate its fields at the top level. Reading only `properties` reports + an `allOf`-composed field as undeclared, which sends an author chasing a warning about a key that is + perfectly valid.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + manifest["spec"] = { + "type": "Spec", + "connection_specification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {}, + "allOf": [{"properties": {"resource": {"type": "string"}}}], + }, + } + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + with caplog.at_level(logging.WARNING): + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert "not declared in the connector spec" not in caplog.text + + +def test_given_an_override_on_a_secret_field_then_the_value_is_registered_for_redaction(): + """The entrypoint builds the secret list from the config the user supplied, so a value substituted + here is unknown to `filter_secrets` - and would print in the clear at a path where the user's own + value prints as `****`.""" + from airbyte_cdk.utils.airbyte_secrets_utils import filter_secrets, update_secrets + + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only-secret"}, + } + manifest["spec"] = _spec_with({"resource": {"type": "string", "airbyte_secret": True}}) + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + update_secrets([]) + try: + with HttpMocker() as http_mocker: + http_mocker.get( + HttpRequest(url="https://api.test.com/check-only-secret"), + HttpResponse(body=json.dumps([{"id": 1}])), + ) + + assert source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG).status == Status.SUCCEEDED + + assert filter_secrets("saw check-only-secret") == "saw ****" + finally: + update_secrets([]) + + +def test_config_overrides_is_published_on_both_check_components(): + """The schema is the contract the Connector Builder and the manifest server read, and the factory + deliberately ignores the field - so nothing at runtime notices if it disappears from the published + surface.""" + schema = yaml.safe_load( + pkgutil.get_data( + "airbyte_cdk.sources.declarative", "declarative_component_schema.yaml" + ).decode() + ) + + for component in ("CheckStream", "CheckDynamicStream"): + assert "config_overrides" in schema["definitions"][component]["properties"] + + assert CheckStreamModel( + type="CheckStream", stream_names=["items"], config_overrides={"a": 1} + ).config_overrides == {"a": 1} + assert CheckDynamicStreamModel( + type="CheckDynamicStream", stream_count=1, config_overrides={"a": 1} + ).config_overrides == {"a": 1} + + +def test_given_a_custom_authenticator_declaring_a_refresh_token_updater_then_it_is_rejected(): + """`refresh_token_updater` is declared on `OAuthAuthenticator` alone, so matching that type would be + enough for the schema as written. `CustomAuthenticator` is matched too because the transformer + injects that type for a `class_name` component, and custom code that declares the field is the shape + most likely to write the config back - the one case of custom code the manifest does name.""" + manifest = deepcopy(_MANIFEST_WITH_CONFIG_DRIVEN_PATH) + manifest["check"] = { + "type": "CheckStream", + "stream_names": ["items"], + "config_overrides": {"resource": "check-only"}, + } + manifest["streams"][0]["retriever"]["requester"]["authenticator"] = { + "type": "CustomAuthenticator", + "class_name": "unit_tests.sources.declarative.test_concurrent_declarative_source_config_overrides.NotBuilt", + "refresh_token_updater": {}, + } + + source = ConcurrentDeclarativeSource( + source_config=manifest, + config=_CONFIG_DRIVEN_PATH_CONFIG, + catalog=None, + state=None, + ) + + # Matched on the guard's own sentence: a failure to import the custom class also mentions + # `refresh_token_updater`, because the message echoes the component definition. + with pytest.raises( + AirbyteTracedException, match="cannot be used by a manifest that declares" + ) as raised: + source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) + assert raised.value.failure_type == FailureType.config_error From 333e8aca7e28e721c2c9b63df9ee3e07cb6d211c Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Thu, 20 Aug 2026 16:43:55 +0300 Subject: [PATCH 09/10] chore(low-code): drop dead code left over from the config_overrides refactor Two leftovers from moving the config_overrides tests into their own module: - `ConcurrentDeclarativeSource._FIELDS_HOLDING_CONFIG_VALUES` had no readers. Only `manifest_reference_resolver` needs that set, and it declares its own module-level copy next to the code that consumes it. - `test_check_stream.py` kept eleven imports that only the moved tests used. `ruff check` did not catch them because this repo does not select F401; CodeQL did. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 3 --- .../declarative/checks/test_check_stream.py | 19 +------------------ 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index 1c88691f6..5354d1793 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -152,9 +152,6 @@ class ConcurrentDeclarativeSource(Source): {"OAuthAuthenticator", "CustomAuthenticator"} ) - # Manifest fields whose values are connector config rather than components. - _FIELDS_HOLDING_CONFIG_VALUES = frozenset({"config_overrides"}) - def __init__( self, catalog: Optional[ConfiguredAirbyteCatalog] = None, diff --git a/unit_tests/sources/declarative/checks/test_check_stream.py b/unit_tests/sources/declarative/checks/test_check_stream.py index 6b97ca782..8bc9571d1 100644 --- a/unit_tests/sources/declarative/checks/test_check_stream.py +++ b/unit_tests/sources/declarative/checks/test_check_stream.py @@ -4,39 +4,22 @@ import json import logging -import pkgutil from copy import deepcopy from typing import Any, Iterable, Mapping, Optional from unittest.mock import MagicMock import pytest import requests -import yaml from jsonschema.exceptions import ValidationError -from airbyte_cdk.entrypoint import AirbyteEntrypoint -from airbyte_cdk.models import ( - AirbyteConnectionStatus, - AirbyteMessage, - ConnectorSpecification, - FailureType, - Status, - Type, -) +from airbyte_cdk.models import Status from airbyte_cdk.sources.declarative.checks.check_stream import CheckStream from airbyte_cdk.sources.declarative.concurrent_declarative_source import ( ConcurrentDeclarativeSource, ) -from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( - CheckDynamicStream as CheckDynamicStreamModel, -) -from airbyte_cdk.sources.declarative.models.declarative_component_schema import ( - CheckStream as CheckStreamModel, -) from airbyte_cdk.sources.streams.core import Stream from airbyte_cdk.sources.streams.http import HttpStream from airbyte_cdk.test.mock_http import HttpMocker, HttpRequest, HttpResponse -from airbyte_cdk.utils.traced_exception import AirbyteTracedException logger = logging.getLogger("test") config = dict() From ce0228e826e123f337f45ccdfac36dde9b9a7925 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Thu, 20 Aug 2026 17:23:29 +0300 Subject: [PATCH 10/10] fix(low-code): classify config_overrides guard failures as system errors All three `config_overrides` guards fire on a manifest authoring mistake, not on anything the end user can correct, so `config_error` put them in the wrong queue and the messages told the user to edit a manifest they cannot see. This follows the precedent set by `_extract_path` in `rate_limited_multiple_token.py`: paths that come from the manifest are a `system_error`. The user-facing `message` now says the connector's manifest is invalid and keeps the detail that identifies the offending keys; the manifest-level remedy moves to `internal_message`, where the connector developer reads it. This does change what `check` emits. `AirbyteEntrypoint.check` re-raises anything that is not a `config_error`, so a rejected manifest now emits the TRACE and exits non-zero instead of reporting a FAILED connection status. That is the intended outcome -- a broken manifest is a connector bug, not a bad connection -- and the entrypoint test is rewritten to pin it, including that the TRACE still carries the guard's message. Also documents in the schema that a `$ref` inside `config_overrides` is left unresolved, which the resolver's exemption made true but nothing recorded. Co-Authored-By: Claude Opus 5 (1M context) --- .../concurrent_declarative_source.py | 47 +++++++++++++------ .../declarative_component_schema.yaml | 4 +- .../models/declarative_component_schema.py | 4 +- ...ent_declarative_source_config_overrides.py | 45 +++++++++--------- 4 files changed, 60 insertions(+), 40 deletions(-) diff --git a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py index 5354d1793..52f83a644 100644 --- a/airbyte_cdk/sources/declarative/concurrent_declarative_source.py +++ b/airbyte_cdk/sources/declarative/concurrent_declarative_source.py @@ -680,11 +680,18 @@ def _raise_on_reserved_override_keys(config_overrides: Mapping[str, Any]) -> Non if reserved: raise AirbyteTracedException( message=( - f"`config_overrides` may not set the reserved key(s) {reserved}. Keys prefixed " - "with `__airbyte` belong to the platform, not to the connector's spec." + f"This connector's manifest is invalid: its check component overrides the " + f"reserved config key(s) {reserved}. Keys prefixed with `__airbyte` belong to " + "the platform, not to the connector's spec. This is a bug in the connector " + "rather than in this connection's settings." ), - internal_message="config_overrides rejected: reserved __airbyte key", - failure_type=FailureType.config_error, + internal_message=( + f"config_overrides rejected: reserved __airbyte key(s) {reserved}. " + "`CheckStream` reads `__airbyte_check_stream_names` out of the config this " + "overlay writes to, so an override there would change which streams a check " + "tests. Use `stream_names` instead." + ), + failure_type=FailureType.system_error, ) @staticmethod @@ -694,11 +701,17 @@ def _raise_on_non_string_override_keys(config_overrides: Mapping[str, Any]) -> N if non_strings: raise AirbyteTracedException( message=( - f"`config_overrides` keys must be strings, but {sorted(map(repr, non_strings))} " - "are not. Quote them in the manifest so they name a field in the connector's spec." + "This connector's manifest is invalid: its check component has " + f"`config_overrides` key(s) {sorted(map(repr, non_strings))} that are not " + "strings, so they cannot name a field in the connector's spec. This is a bug " + "in the connector rather than in this connection's settings." + ), + internal_message=( + "config_overrides rejected: non-string key(s) " + f"{sorted(map(repr, non_strings))}. Quote them in the manifest so they name a " + "field in the connector's spec." ), - internal_message="config_overrides rejected: non-string key", - failure_type=FailureType.config_error, + failure_type=FailureType.system_error, ) def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> None: @@ -713,15 +726,19 @@ def _raise_if_config_is_persisted(self, config_overrides: Mapping[str, Any]) -> return raise AirbyteTracedException( message=( - "`config_overrides` cannot be used by a manifest that declares a " - "`refresh_token_updater`. A token refresh during `check` emits the whole config it was " - "handed as a CONNECTOR_CONFIG control message, which the platform persists, so the " - f"check-only override(s) {sorted(config_overrides)} would be saved as this connection's " - "config and applied to every later sync. Remove `config_overrides`, or drop the " + "This connector's manifest is invalid: `config_overrides` cannot be used by a " + "manifest that declares a `refresh_token_updater`. A token refresh during `check` " + "emits the whole config it was handed as a CONNECTOR_CONFIG control message, which " + f"the platform persists, so the check-only override(s) {sorted(config_overrides)} " + "would be saved as this connection's config and applied to every later sync. This " + "is a bug in the connector rather than in this connection's settings." + ), + internal_message=( + "config_overrides rejected: manifest persists config via refresh_token_updater. " + "Remove `config_overrides` from the check component, or drop the " "`refresh_token_updater`." ), - internal_message="config_overrides rejected: manifest persists config via refresh_token_updater", - failure_type=FailureType.config_error, + failure_type=FailureType.system_error, ) @staticmethod diff --git a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml index 4b22b5c66..016e78bee 100644 --- a/airbyte_cdk/sources/declarative/declarative_component_schema.yaml +++ b/airbyte_cdk/sources/declarative/declarative_component_schema.yaml @@ -560,7 +560,7 @@ definitions: "$ref": "#/definitions/DynamicStreamCheckConfig" config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, a `$ref` inside them is not resolved, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. type: object additionalProperties: true examples: @@ -606,7 +606,7 @@ definitions: default: true config_overrides: title: Config Overrides - description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. + description: Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, a `$ref` inside them is not resolved, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync. type: object additionalProperties: true examples: diff --git a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py index 515eb5d1b..d09848a23 100644 --- a/airbyte_cdk/sources/declarative/models/declarative_component_schema.py +++ b/airbyte_cdk/sources/declarative/models/declarative_component_schema.py @@ -79,7 +79,7 @@ class CheckDynamicStream(BaseModel): ) config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, a `$ref` inside them is not resolved, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) @@ -1802,7 +1802,7 @@ class CheckStream(BaseModel): dynamic_streams_check_configs: Optional[List[DynamicStreamCheckConfig]] = None config_overrides: Optional[Dict[str, Any]] = Field( None, - description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", + description="Values overlaid onto the connector config for the duration of the check operation only. Use this when a check must behave differently from a sync - for example a shorter rate limit wait budget, so that check fails fast with a clear message instead of sleeping until the quota resets. Keys should be fields declared in the connector's spec. Values are used as-is. They are not interpolated, a `$ref` inside them is not resolved, they replace a nested object rather than deep-merging into it, and they are applied after config migrations and transformations have run, so a field derived from an overridden field is not recomputed. Keys must be strings, and two combinations are rejected outright. Keys prefixed with `__airbyte` belong to the platform rather than to the connector's spec. And a manifest that declares a `refresh_token_updater` cannot use this field at all, because a token refresh during check emits the whole config it was handed as a CONNECTOR_CONFIG control message, which the platform persists - so a check-only override would become the connection's saved config and apply to every later sync.", examples=[{"max_waiting_time": 0}, {"page_size": 1}], title="Config Overrides", ) diff --git a/unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py b/unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py index dca9b76b6..7c9492a20 100644 --- a/unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py +++ b/unit_tests/sources/declarative/test_concurrent_declarative_source_config_overrides.py @@ -370,7 +370,7 @@ def test_given_refresh_token_updater_when_config_overrides_then_manifest_is_reje with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error + assert raised.value.failure_type == FailureType.system_error def test_given_no_refresh_token_updater_when_config_overrides_then_manifest_is_accepted(): @@ -491,7 +491,7 @@ def test_given_refresh_token_updater_behind_a_ref_then_manifest_is_rejected(): with pytest.raises(AirbyteTracedException, match="refresh_token_updater") as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error + assert raised.value.failure_type == FailureType.system_error def test_given_refresh_token_updater_without_config_overrides_then_nothing_is_rejected(): @@ -587,7 +587,7 @@ def test_given_an_airbyte_reserved_override_key_then_the_manifest_is_rejected(): with pytest.raises(AirbyteTracedException, match="__airbyte_check_stream_names") as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error + assert raised.value.failure_type == FailureType.system_error def _spec_with(properties): @@ -601,11 +601,13 @@ def _spec_with(properties): } -def test_given_a_reserved_key_when_check_through_the_entrypoint_then_a_failed_status_is_emitted(): - """The guards exist to hand a manifest author an actionable sentence. `AirbyteEntrypoint.check` - catches `AirbyteTracedException` and nothing else, so a bare `ValueError` would escape `run()`, be - re-wrapped as a generic system error, and emit no CONNECTION_STATUS at all - throwing away the very - message the guard was written to deliver.""" +def test_given_a_reserved_key_when_check_through_the_entrypoint_then_a_trace_is_emitted_and_it_raises(): + """The guards fire on a manifest authoring mistake, so they raise `system_error`, and + `AirbyteEntrypoint.check` treats anything other than `config_error` as exceptional: it emits the + TRACE and then re-raises rather than reporting a FAILED connection status. That is deliberate -- a + broken manifest is a connector bug and should exit non-zero, not be reported to the user as a bad + connection. What must not regress is the TRACE: a bare `ValueError` would escape `run()` entirely + and throw away the message the guard was written to deliver.""" source = _source_with_check_component( { "type": "CheckStream", @@ -615,18 +617,19 @@ def test_given_a_reserved_key_when_check_through_the_entrypoint_then_a_failed_st ) entrypoint = AirbyteEntrypoint(source) - messages = list( - entrypoint.check( + messages = [] + with pytest.raises(AirbyteTracedException) as raised: + for message in entrypoint.check( ConnectorSpecification(connectionSpecification={}), _CONFIG_DRIVEN_PATH_CONFIG - ) - ) + ): + messages.append(message) - statuses = [ - message.connectionStatus for message in messages if message.type == Type.CONNECTION_STATUS - ] - assert len(statuses) == 1 - assert statuses[0].status == Status.FAILED - assert "__airbyte_check_stream_names" in statuses[0].message + assert raised.value.failure_type == FailureType.system_error + traces = [message.trace for message in messages if message.type == Type.TRACE] + assert len(traces) == 1 + assert traces[0].error.failure_type == FailureType.system_error + assert "__airbyte_check_stream_names" in traces[0].error.message + assert not [message for message in messages if message.type == Type.CONNECTION_STATUS] def test_given_a_config_override_shaped_like_a_reference_then_it_stays_a_literal(): @@ -793,9 +796,9 @@ def test_given_non_string_override_keys_then_the_manifest_is_rejected_cleanly(): } ) - with pytest.raises(AirbyteTracedException, match="must be strings") as raised: + with pytest.raises(AirbyteTracedException, match="are not strings") as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error + assert raised.value.failure_type == FailureType.system_error def test_given_a_spec_composed_with_all_of_then_no_spurious_warning_is_logged(caplog): @@ -922,4 +925,4 @@ def test_given_a_custom_authenticator_declaring_a_refresh_token_updater_then_it_ AirbyteTracedException, match="cannot be used by a manifest that declares" ) as raised: source.check(logger, _CONFIG_DRIVEN_PATH_CONFIG) - assert raised.value.failure_type == FailureType.config_error + assert raised.value.failure_type == FailureType.system_error