From c4a19656c184444a58a69911452b90df71da2d81 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 15:26:53 +0300 Subject: [PATCH 1/4] feat(standard-tests): lint spec structure, form rendering and OAuth config paths Port CAT's structural spec checks into the spec tests (`test_spec` and `test_docker_image_build_and_spec`), next to the `airbyte_secret` marking lint. Every lint reports each problem with its JSON pointer and never prints a value. Enforced: valid JSON Schema, refs resolved, additionalProperties is true, oneOf usage, enum usage, array items, property types, display_type, duplicate order, nested group and OAuth config paths. Advisory, reported as the new `SpecLintWarning`: forbidden keywords and object not empty. Their remaining violators build their specs in code and cannot be fixed without the spec backward-compatibility check calling the fix breaking. A lint that crashes on an unexpected spec is reported as a problem of that lint instead of raising. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/standard_tests/__init__.py | 6 + airbyte_cdk/test/standard_tests/_spec_lint.py | 631 +++++++++++++++++- cdk-migrations.md | 37 + unit_tests/test/test_spec_lint.py | 34 + unit_tests/test/test_spec_structural_lints.py | 572 ++++++++++++++++ 5 files changed, 1273 insertions(+), 7 deletions(-) create mode 100644 unit_tests/test/test_spec_structural_lints.py diff --git a/airbyte_cdk/test/standard_tests/__init__.py b/airbyte_cdk/test/standard_tests/__init__.py index e99712ae2..6debc4090 100644 --- a/airbyte_cdk/test/standard_tests/__init__.py +++ b/airbyte_cdk/test/standard_tests/__init__.py @@ -34,10 +34,15 @@ class TestSuiteSourcePokeAPI(standard_tests.DeclarativeSourceTestSuite): airbyte-cdk connector test --pytest-arg=-Werror::airbyte_cdk.test.standard_tests.DiscoveredCatalogWarning ``` +The spec tests lint the spec in the same way: problems found by spec lints that are not enforced +yet are reported as a `SpecLintWarning`, which `-W error::...SpecLintWarning` also turns into a +failure. + ''' from airbyte_cdk.test.models.scenario import ConnectorTestScenario from airbyte_cdk.test.standard_tests._catalog_checks import DiscoveredCatalogWarning +from airbyte_cdk.test.standard_tests._spec_lint import SpecLintWarning from airbyte_cdk.test.standard_tests.connector_base import ConnectorTestSuiteBase from airbyte_cdk.test.standard_tests.declarative_sources import ( DeclarativeSourceTestSuite, @@ -52,4 +57,5 @@ class TestSuiteSourcePokeAPI(standard_tests.DeclarativeSourceTestSuite): "DestinationTestSuiteBase", "DiscoveredCatalogWarning", "SourceTestSuiteBase", + "SpecLintWarning", ] diff --git a/airbyte_cdk/test/standard_tests/_spec_lint.py b/airbyte_cdk/test/standard_tests/_spec_lint.py index b2a0ce6f7..09214111d 100644 --- a/airbyte_cdk/test/standard_tests/_spec_lint.py +++ b/airbyte_cdk/test/standard_tests/_spec_lint.py @@ -10,18 +10,23 @@ import json import re -from collections.abc import Iterable, Iterator, Mapping +import warnings +from collections.abc import Callable, Iterable, Iterator, Mapping from dataclasses import dataclass from typing import Any from urllib.parse import quote +import jsonschema + from airbyte_cdk.models import ( AirbyteMessage, AirbyteMessageSerializer, ConnectorSpecification, + ConnectorSpecificationSerializer, Type, ) from airbyte_cdk.test.entrypoint_wrapper import EntrypointOutput +from airbyte_cdk.test.standard_tests._catalog_checks import _iter_subschemas SECRET_PROPERTY_NAMES = frozenset( { @@ -352,6 +357,606 @@ def _has_unmasked_variant_marking(schema: Mapping[str, Any], under_unmasked: boo return False +# Structural, presentational and OAuth lints, ported from CAT's `TestSpec`. + + +class SpecLintWarning(UserWarning): + """A spec problem that the standard tests report but do not enforce yet. + + To make these problems fail the test, turn the warning into an error, either with + `pytest -W error::airbyte_cdk.test.standard_tests.SpecLintWarning` or with + `airbyte-cdk connector test --pytest-arg=-Werror::airbyte_cdk.test.standard_tests.SpecLintWarning`. + """ + + +SPEC_REFERENCE_URL = ( + "https://docs.airbyte.com/platform/connector-development/connector-specification-reference" +) + +FORBIDDEN_SPEC_KEYWORDS = ( + "not", + "anyOf", + "patternProperties", + "prefixItems", + "allOf", + "if", + "then", + "else", + "dependentSchemas", + "dependentRequired", +) +"""JSON Schema keywords that the connector setup form cannot render.""" + +ARRAY_ITEM_TYPES = frozenset({"object", "string", "number", "integer"}) +"""Item types the connector setup form can render as an array field, besides an `enum`.""" + +DISPLAY_TYPES = frozenset({"dropdown", "radio"}) +"""Values `display_type` may take on a field with `oneOf` variants.""" + + +def _schema_nodes(schema: Any) -> Iterator[tuple[tuple[str, ...], Mapping[str, Any]]]: + """Yield `(path, subschema)` for `schema` and every subschema in it. + + Only keywords that hold subschemas are followed, so a property named `type`, `enum` or + `not` is never mistaken for a keyword, and values such as `default` or `examples` are + never read as schemas. + """ + yield from _iter_subschemas(schema) + + +def _spec_pointer(path: Iterable[str]) -> str: + """Render a schema path as the JSON pointer used in failure messages, `` `/` `` at the root.""" + return "`/" + "/".join(_pointer_token(segment) for segment in path) + "`" + + +def _is_property(path: tuple[str, ...]) -> bool: + return len(path) >= 2 and path[-2] == "properties" + + +def _is_single_valued(schema: Any) -> bool: + """Return whether a property can hold one value only: a `const`, or an `enum` of one item.""" + if not isinstance(schema, Mapping): + return False + enum = schema.get("enum") + return "const" in schema or (isinstance(enum, list) and len(enum) == 1) + + +def find_oneof_usage_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `oneOf` that the setup form cannot render as a choice of variants. + + Ported from CAT's `test_oneof_usage`. The schema that declares `oneOf` must be an object, + and every variant must be an object with `properties`. The variants must share exactly one + discriminator: a property that sets `const` in every variant or, failing that, one that + sets a single-item `enum` in every variant. The setup form picks the variant by that + value, so a `const` discriminator may set `default` and `enum` only to the same value, an + `enum` discriminator must set its one item as `default` (the form reads it as the + `const`), and no two variants may share a value. + + Unlike CAT, a property whose `enum` has several items is not a discriminator candidate, + since it cannot identify a variant (such as a `compression_codec` next to `format_type`). + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + variants = schema.get("oneOf") + if not isinstance(variants, list): + continue + where = _spec_pointer((*path, "oneOf")) + if schema.get("type") != "object": + errors.append( + f"{where} is declared on a schema that is not `type: object`; the setup form " + "renders `oneOf` only as a choice between object variants." + ) + variant_properties: list[Mapping[str, Any]] = [] + for index, variant in enumerate(variants): + properties = variant.get("properties") if isinstance(variant, Mapping) else None + if not isinstance(properties, Mapping): + errors.append( + f"{_spec_pointer((*path, 'oneOf', str(index)))} has no `properties`; every " + "`oneOf` variant must be an object with properties." + ) + continue + variant_properties.append(properties) + if not variants or len(variant_properties) != len(variants): + continue + + common = set.intersection(*(set(properties) for properties in variant_properties)) + candidates = sorted( + name + for name in common + if all(_is_single_valued(properties[name]) for properties in variant_properties) + ) + const_candidates = [ + name + for name in candidates + if all("const" in properties[name] for properties in variant_properties) + ] + if len(const_candidates) == 1: + discriminator = const_candidates[0] + elif not const_candidates and len(candidates) == 1: + discriminator = candidates[0] + else: + found = const_candidates or candidates + detail = ", ".join(f"`{name}`" for name in found) if found else "none" + errors.append( + f"{where} variants must share exactly one property that sets `const` in every " + f"variant, so the setup form can tell them apart (found: {detail})." + ) + continue + + values: list[Any] = [] + for index, properties in enumerate(variant_properties): + discriminator_schema = properties[discriminator] + discriminator_where = _spec_pointer( + (*path, "oneOf", str(index), "properties", discriminator) + ) + if "const" in discriminator_schema: + value = discriminator_schema["const"] + if "default" in discriminator_schema and not _same_json( + discriminator_schema["default"], value + ): + errors.append( + f"{discriminator_where} sets a `default` that differs from its `const`." + ) + if "enum" in discriminator_schema and not _same_json( + discriminator_schema["enum"], [value] + ): + errors.append( + f"{discriminator_where} sets an `enum` other than a single item equal " + "to its `const`." + ) + elif "default" in discriminator_schema and _same_json( + discriminator_schema["enum"], [discriminator_schema["default"]] + ): + value = discriminator_schema["default"] + else: + errors.append( + f"{discriminator_where} must set `const`, or set its single `enum` item as " + "its `default`." + ) + continue + values.append(value) + if any(_count_json(values, value) > 1 for value in values): + errors.append( + f"{where} variants share a value of the discriminator `{discriminator}`, so " + "the setup form cannot tell them apart." + ) + return errors + + +def _same_json(left: Any, right: Any) -> bool: + return json.dumps(left, sort_keys=True) == json.dumps(right, sort_keys=True) + + +def _count_json(values: list[Any], value: Any) -> int: + return sum(_same_json(item, value) for item in values) + + +def find_enum_usage_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `enum` that lists the same value more than once. + + Ported from CAT's `test_enum_usage`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + enum = schema.get("enum") + if not isinstance(enum, list): + continue + seen: dict[str, int] = {} + for index, value in enumerate(enum): + key = json.dumps(value, sort_keys=True) + if key in seen: + errors.append( + f"{_spec_pointer((*path, 'enum'))} lists the same value at positions " + f"{seen[key]} and {index}." + ) + else: + seen[key] = index + return errors + + +def find_forbidden_keyword_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per JSON Schema keyword that the setup form cannot render. + + Ported from CAT's `test_forbidden_complex_types`: see `FORBIDDEN_SPEC_KEYWORDS`. + """ + return [ + f"{_spec_pointer((*path, keyword))} uses `{keyword}`, which the connector setup form " + "does not support." + for path, schema in _schema_nodes(connection_specification) + for keyword in FORBIDDEN_SPEC_KEYWORDS + if keyword in schema + ] + + +def find_array_type_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per array whose items the setup form cannot render. + + Ported from CAT's `test_array_type`: `items` must be a single schema, of type `object`, + `string`, `number` or `integer`, or with an `enum`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + if schema.get("type") != "array" or "items" not in schema: + continue + items = schema["items"] + where = _spec_pointer((*path, "items")) + if not isinstance(items, Mapping): + errors.append(f"{where} must be a single schema, not a list of item schemas.") + elif items.get("type") not in ARRAY_ITEM_TYPES and "enum" not in items: + errors.append( + f"{where} must be of type `object`, `string`, `number` or `integer`, or set " + "an `enum`." + ) + return errors + + +def find_empty_object_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per object that declares no properties and no `oneOf` variants. + + Ported from CAT's `test_object_not_empty`: the setup form shows nothing for such an object. + The root schema may be an empty object, for a connector that takes no configuration. + """ + return [ + f"{_spec_pointer(path)} is an object with no `properties` and no `oneOf`, so the setup " + "form cannot show it. Add its properties or remove it." + for path, schema in _schema_nodes(connection_specification) + if path + and schema.get("type") == "object" + and "oneOf" not in schema + and not schema.get("properties") + ] + + +def find_property_type_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per property whose `type` lists types the setup form cannot render. + + Ported from CAT's `test_property_type_is_not_array`: a list of types must hold one type, + or one type followed by `null`, such as `[string, null]`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + types = schema.get("type") + if not _is_property(path) or not isinstance(types, list): + continue + if len(types) not in (1, 2) or (len(types) == 2 and types[1] != "null"): + errors.append( + f"{_spec_pointer((*path, 'type'))} must be a single type, or a type followed by " + "`null`, such as `[string, null]`." + ) + return errors + + +def _iter_keys(value: Any, path: tuple[str, ...] = ()) -> Iterator[tuple[tuple[str, ...], str]]: + """Yield `(path, key)` for every mapping key anywhere in a JSON-like value.""" + if isinstance(value, Mapping): + for key, item in value.items(): + yield path, str(key) + yield from _iter_keys(item, (*path, str(key))) + elif isinstance(value, list): + for index, item in enumerate(value): + yield from _iter_keys(item, (*path, str(index))) + + +def find_unresolved_ref_errors(spec: Mapping[str, Any]) -> list[str]: + """Return one error per `$ref` left in the emitted spec. + + Ported from CAT's `test_defined_refs_exist_in_json_spec_file`. The platform does not + resolve references in a spec, so the connector must emit it with every `$ref` resolved. + `spec` is the whole SPEC message payload, so `advanced_auth` is checked too. + """ + return [ + f"{_spec_pointer((*path, key))} is an unresolved reference; emit the spec with every " + "`$ref` resolved." + for path, key in _iter_keys(spec) + if key == "$ref" + ] + + +def find_additional_properties_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `additionalProperties` that is set to anything but `true`. + + Ported from CAT's `test_additional_properties_is_true`. With `false` or a schema, removing + a property in a later version makes every existing config that sets it invalid. + """ + return [ + f"{_spec_pointer((*path, 'additionalProperties'))} must be `true` when set; any other " + "value makes existing configs invalid as soon as a property is removed." + for path, schema in _schema_nodes(connection_specification) + if "additionalProperties" in schema and schema["additionalProperties"] is not True + ] + + +def find_display_type_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per misused `display_type`. + + Ported from CAT's `test_display_type`: `display_type` belongs on a field with `oneOf` + variants, and must be `dropdown` or `radio`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + if "display_type" not in schema: + continue + where = _spec_pointer((*path, "display_type")) + if "oneOf" not in schema: + errors.append(f"{where} is set on a field without `oneOf` variants.") + if schema["display_type"] not in DISPLAY_TYPES: + errors.append(f"{where} must be `dropdown` or `radio`.") + return errors + + +def find_duplicate_order_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `order` value used twice among the properties of one object. + + Ported from CAT's `test_duplicate_order`. Properties in different `group`s may share an + `order`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + properties = schema.get("properties") + if schema.get("type") != "object" or not isinstance(properties, Mapping): + continue + used: dict[tuple[str, str], str] = {} + for name, prop in properties.items(): + if not isinstance(prop, Mapping) or "order" not in prop: + continue + key = (str(prop.get("group", "")), json.dumps(prop["order"])) + if key in used: + errors.append( + f"{_spec_pointer((*path, 'properties', name, 'order'))} repeats the " + f"`order` of `{used[key]}`" + (f" in group `{key[0]}`." if key[0] else ".") + ) + else: + used[key] = name + return errors + + +def find_nested_group_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `group` set below the top-level properties. + + Ported from CAT's `test_nested_group`: the setup form groups top-level fields only. + """ + return [ + f"{_spec_pointer((*path, 'group'))} is set below the top level; only top-level " + "properties can be grouped." + for path, schema in _schema_nodes(connection_specification) + if "group" in schema and path and not (len(path) == 2 and path[0] == "properties") + ] + + +def _config_paths(schema: Any, prefix: tuple[str, ...] = ()) -> Iterator[tuple[str, ...]]: + """Yield the path of every property a config can set, through `oneOf`/`anyOf` variants. + + An array's items add a `[]` segment, as in CAT's `get_expected_schema_structure`. + """ + if not isinstance(schema, Mapping): + return + for keyword in _COMBINATORS: + for variant in _variants(schema, (keyword,)): + yield from _config_paths(variant, prefix) + properties = schema.get("properties") + if isinstance(properties, Mapping): + for name, prop in properties.items(): + yield (*prefix, str(name)) + yield from _config_paths(prop, (*prefix, str(name))) + items = schema.get("items") + if isinstance(items, Mapping): + yield from _config_paths(items, (*prefix, "[]")) + + +def find_oauth_parameter_errors(spec: Mapping[str, Any]) -> list[str]: + """Return one error per `advanced_auth` config path that the connection spec lacks. + + Ported from CAT's `test_oauth_flow_parameters`. `predicate_key` and every + `path_in_connector_config` in the OAuth input and output specifications must name a + property of `connectionSpecification`, or the platform reads or writes a config field + that does not exist. + """ + advanced_auth = spec.get("advanced_auth") + if not isinstance(advanced_auth, Mapping): + return [] + known = set(_config_paths(spec.get("connectionSpecification") or {})) + referenced: list[tuple[str, list[Any]]] = [] + if isinstance(advanced_auth.get("predicate_key"), list): + referenced.append(("`/advanced_auth/predicate_key`", advanced_auth["predicate_key"])) + oauth = advanced_auth.get("oauth_config_specification") + if isinstance(oauth, Mapping): + for section in ( + "oauth_user_input_from_connector_config_specification", + "complete_oauth_output_specification", + "complete_oauth_server_output_specification", + ): + properties = (oauth.get(section) or {}).get("properties") + if not isinstance(properties, Mapping): + continue + for name, prop in properties.items(): + if isinstance(prop, Mapping) and isinstance( + prop.get("path_in_connector_config"), list + ): + referenced.append( + ( + _spec_pointer( + ( + "advanced_auth", + "oauth_config_specification", + section, + "properties", + name, + "path_in_connector_config", + ) + ), + prop["path_in_connector_config"], + ) + ) + return [ + f"{where} names the config path `/{'/'.join(str(part) for part in config_path)}`, " + "which is not a property of `connectionSpecification`." + for where, config_path in referenced + if tuple(str(part) for part in config_path) not in known + ] + + +def find_invalid_schema_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per place where the spec is not a valid JSON Schema. + + The spec is checked against the meta-schema of the draft it declares in `$schema`, draft 7 + by default. CAT only found this through `test_config_match_spec`, when a scenario config + was validated against an invalid spec. + """ + validator_class = jsonschema.validators.validator_for( + connection_specification, default=jsonschema.Draft7Validator + ) + return [ + f"{_spec_pointer(map(str, error.path))} is not valid JSON Schema: it fails the " + f"`{error.validator}` rule of the meta-schema." + for error in validator_class(validator_class.META_SCHEMA).iter_errors( + connection_specification + ) + ] + + +@dataclass(frozen=True) +class SpecLint: + """A named credential-free assertion on a connector spec.""" + + name: str + rule: str + """What the lint requires and why, printed above its problems.""" + run: Callable[[Mapping[str, Any]], list[str]] + """Takes the whole SPEC payload (`connectionSpecification`, `advanced_auth`, ...).""" + enforced: bool = True + + +def _on_connection_specification( + check: Callable[[Mapping[str, Any]], list[str]], +) -> Callable[[Mapping[str, Any]], list[str]]: + def run(spec: Mapping[str, Any]) -> list[str]: + connection_specification = spec.get("connectionSpecification") + if not isinstance(connection_specification, Mapping): + return [] + return check(connection_specification) + + return run + + +SPEC_LINTS: tuple[SpecLint, ...] = ( + SpecLint( + "secret marking", + "A property that holds a credential must set `airbyte_secret: true` so the platform " + "masks it in the UI and logs and stores it in the secrets manager.", + _on_connection_specification(find_secret_marking_errors), + ), + SpecLint( + "valid JSON Schema", + "The spec must be a valid JSON Schema, or the platform cannot validate configs with it.", + _on_connection_specification(find_invalid_schema_errors), + ), + SpecLint( + "refs resolved", + "The platform does not resolve `$ref`, so the emitted spec must contain none.", + find_unresolved_ref_errors, + ), + SpecLint( + "additionalProperties is true", + "`additionalProperties` must be `true` when set, so that removing a property later " + "does not invalidate saved configs.", + _on_connection_specification(find_additional_properties_errors), + ), + SpecLint( + "oneOf usage", + "The setup form renders `oneOf` as a choice of object variants told apart by one " + "shared `const` property.", + _on_connection_specification(find_oneof_usage_errors), + ), + SpecLint( + "enum usage", + "An `enum` must not list the same value twice.", + _on_connection_specification(find_enum_usage_errors), + ), + SpecLint( + "array items", + "The setup form renders arrays of objects, strings, numbers, integers or enum values only.", + _on_connection_specification(find_array_type_errors), + ), + SpecLint( + "property types", + "The setup form renders a property's first type, so a list of types must be one type, " + "optionally followed by `null`.", + _on_connection_specification(find_property_type_errors), + ), + SpecLint( + "display_type", + "`display_type` belongs on a field with `oneOf` variants and must be `dropdown` or " + "`radio`.", + _on_connection_specification(find_display_type_errors), + ), + SpecLint( + "duplicate order", + "The setup form orders fields by `order`, which must be unique within one object and " + "group.", + _on_connection_specification(find_duplicate_order_errors), + ), + SpecLint( + "nested group", + "The setup form groups top-level fields only.", + _on_connection_specification(find_nested_group_errors), + ), + SpecLint( + "OAuth config paths", + "Every config path in `advanced_auth` must name a property of the connection " + "specification.", + find_oauth_parameter_errors, + ), + # Not enforced yet. `source-facebook-marketing` (a single-item `allOf` that pydantic emits + # for an enum field) and `destination-ragie` (pydantic-v2 `Optional` fields as `anyOf`) + # build their specs in code. Inlining those schemas accepts exactly the same configs, but the + # spec backward-compatibility check reports it as breaking. + SpecLint( + "forbidden keywords", + "The connector setup form cannot render these JSON Schema keywords.", + _on_connection_specification(find_forbidden_keyword_errors), + enforced=False, + ), + # Not enforced yet: `source-smoke-test` builds its spec in PyAirbyte, outside the monorepo, + # and its free-form `json_schema` and `records` objects cannot gain properties compatibly. + SpecLint( + "object not empty", + "The setup form cannot show an object with no `properties` and no `oneOf`.", + _on_connection_specification(find_empty_object_errors), + enforced=False, + ), +) +"""The lints `assert_spec_is_valid` runs, in the order their problems are reported.""" + + +def find_spec_problems( + spec: Mapping[str, Any], + lints: Iterable[SpecLint] = SPEC_LINTS, +) -> dict[SpecLint, list[str]]: + """Run each lint on a SPEC payload and return the problems of every failing lint. + + `spec` is the SPEC message's `spec` object as plain JSON. A lint that raises is reported + as a problem of that lint, so that a lint which is not enforced cannot fail the test by + crashing on a spec it did not anticipate. + """ + results: dict[SpecLint, list[str]] = {} + for lint in lints: + try: + problems = lint.run(spec) + except Exception as error: # Reported, not raised; see the docstring. + problems = [f"The lint could not run: {type(error).__name__}."] + if problems: + results[lint] = problems + return results + + +def format_spec_problems(problems: Mapping[SpecLint, list[str]]) -> str: + """Render problems grouped by lint, for an assertion or warning message.""" + return "\n".join( + f"[{lint.name}] {lint.rule}\n" + "\n".join(f" - {problem}" for problem in found) + for lint, found in problems.items() + ) + + def get_single_spec(result: EntrypointOutput, *, connector_name: str) -> ConnectorSpecification: """Assert that `spec` emitted exactly one SPEC message, and return its specification.""" spec_messages = result.spec_messages @@ -367,12 +972,24 @@ def get_single_spec(result: EntrypointOutput, *, connector_name: str) -> Connect def assert_spec_is_valid(spec: ConnectorSpecification, *, connector_name: str) -> None: - """Assert that a connector spec passes every credential-free lint.""" - errors = find_secret_marking_errors(spec.connectionSpecification) - assert not errors, ( - f"The spec of connector '{connector_name}' marks secrets incorrectly. A property that " - "holds a credential must set `airbyte_secret: true` so the platform masks it in the UI " - "and logs and stores it in the secrets manager:\n" + "\n".join(errors) + """Assert that a connector spec passes every enforced credential-free lint. + + Problems found by lints that are not enforced yet (see `SPEC_LINTS`) are reported as a + `SpecLintWarning` instead. No lint prints a config value: a spec holds none. + """ + problems = find_spec_problems(ConnectorSpecificationSerializer.dump(spec)) + advisory = {lint: found for lint, found in problems.items() if not lint.enforced} + if advisory: + warnings.warn( + f"The spec of connector '{connector_name}' has problems that will fail this test in " + f"a future CDK release:\n{format_spec_problems(advisory)}", + category=SpecLintWarning, + stacklevel=2, + ) + enforced = {lint: found for lint, found in problems.items() if lint.enforced} + assert not enforced, ( + f"The spec of connector '{connector_name}' fails the spec lint. See " + f"{SPEC_REFERENCE_URL} for the rules.\n{format_spec_problems(enforced)}" ) diff --git a/cdk-migrations.md b/cdk-migrations.md index ae3781c9b..e348cda6c 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -1,5 +1,42 @@ # CDK Migration Guide +## Upgrading to the next minor release (unreleased) + +This release adds structural lints to the spec tests (`test_spec` and `test_docker_image_build_and_spec`), ported from the spec checks of the Connector Acceptance Tests (CAT). They run on the spec that `spec` emits, need no credentials, and report every problem with the JSON pointer of the place in the spec, never a value. The release is a minor one, since only the test harness changes. Every connector in the monorepo that failed an enforced lint was fixed before this release. + +### Enforced spec lints + +The spec tests now fail when the emitted spec breaks one of these rules. Each failure names the lint, its rule and the pointer of every problem, and links the [connector specification reference](https://docs.airbyte.com/platform/connector-development/connector-specification-reference). + +| Lint | Rule | How to fix it | +|---|---|---| +| valid JSON Schema | `connectionSpecification` is valid against the meta-schema of the draft it declares in `$schema`, draft 7 by default. | Fix the reported keyword, such as a `type` that is not a JSON Schema type or a `required` that is not a list. | +| refs resolved | The emitted spec contains no `$ref`, in `connectionSpecification` or `advanced_auth`. | Emit the spec with every reference resolved (pydantic: resolve `$defs` before emitting it). | +| additionalProperties is true | `additionalProperties` is `true` wherever it is set. With `false` or a schema, removing a property later invalidates every saved config that sets it. | Set it to `true`, or remove it. Relaxing `false` to `true` is backward compatible. | +| oneOf usage | A schema with `oneOf` is `type: object`, every variant has `properties`, and the variants share exactly one discriminator: a property that sets `const` in every variant or, failing that, a single-item `enum` with that item as `default`. A `const` discriminator may set `default` and `enum` only to its `const`, and no two variants share a discriminator value. | Add a shared `const` property, such as `auth_type`, to each variant. | +| enum usage | An `enum` does not list the same value twice. | Remove the duplicate. | +| array items | An array's `items` is one schema, of type `object`, `string`, `number` or `integer`, or with an `enum`. | Change the item type. | +| property types | A property whose `type` is a list holds one type, or one type followed by `null`. The setup form renders the first type only. | Reorder `[null, string]` to `[string, null]`. | +| display_type | `display_type` is set only on a field with `oneOf` variants, to `dropdown` or `radio`. | Remove it or fix its value. | +| duplicate order | No two properties of one object (and one `group`) share an `order`. | Give each property a unique `order`. `order` is an annotation, so changing it is backward compatible. | +| nested group | `group` is set only on top-level properties. | Move the `group` to the top-level property. | +| OAuth config paths | `advanced_auth.predicate_key` and every `path_in_connector_config` in `oauth_config_specification` name a property of `connectionSpecification` (through `oneOf` variants; `[]` for array items). | Fix the path, or add the property. | + +### Advisory spec lints + +These lints are reported as a `SpecLintWarning` and do not fail the test yet; a later release will enforce them: + +- **forbidden keywords**: the spec uses a keyword the setup form cannot render: `not`, `anyOf`, `allOf`, `patternProperties`, `prefixItems`, `if`, `then`, `else`, `dependentSchemas` or `dependentRequired`. Pydantic emits `allOf` for a field typed as an enum and `anyOf` for an `Optional` field (pydantic v2); inline the referenced schema, and for an optional field drop the `null` branch and the field from `required`. +- **object not empty**: a nested `type: object` declares no `properties` and no `oneOf`, so the setup form shows nothing for it. Declare its properties, or remove it. + +To make the warnings fail the test now, run `airbyte-cdk connector test --pytest-arg=-Werror::airbyte_cdk.test.standard_tests.SpecLintWarning`, or pass `-W error::airbyte_cdk.test.standard_tests.SpecLintWarning` to `pytest`. + +### CAT spec checks that are not ported + +- `test_date_pattern` (CAT only warned) is dropped: it asked for `MM-DD-YYYY` on `format: date` fields, which contradicts the setup form's date picker, and adding a `pattern` narrows the accepted configs. +- `test_date_format` and `test_oauth_is_default_method` are handed to the connector QA checks (`airbyte-ops local connector qa`): each has a known violator that cannot be fixed compatibly or chose its default deliberately, so it needs a per-connector exception that the standard tests do not offer. +- `test_image_labels` and `test_image_environment_variables` are dropped: `airbyte-cdk image build` sets the `io.airbyte.name` and `io.airbyte.version` labels from `metadata.yaml`, and the shared connector Dockerfiles set `AIRBYTE_ENTRYPOINT` to the entrypoint, so a connector cannot get them wrong. + ## Upgrading to 7.36.0 This release of the CDK adds checks to the Standard Tests (`airbyte-cdk connector test` and `airbyte-cdk image test`) and changes which commands they run for the configs listed in `acceptance-test-config.yml`. There is no change to how connectors run. The release is a minor one, since only the test harness changes, but the connector test suites of connectors that do not satisfy the new expectations start failing. The connector monorepo installs the `airbyte-cdk` CLI unpinned (`uv tool install --upgrade 'airbyte-cdk[dev]'`), so connector CI picks the new behaviour up with this release, without a per-connector CDK bump. diff --git a/unit_tests/test/test_spec_lint.py b/unit_tests/test/test_spec_lint.py index e36704c79..5a49f8b2a 100644 --- a/unit_tests/test/test_spec_lint.py +++ b/unit_tests/test/test_spec_lint.py @@ -1303,3 +1303,37 @@ def run_docker_command(cmd: list[str], **kwargs: Any) -> subprocess.CompletedPro monkeypatch.setattr(docker_utils, "run_docker_command", run_docker_command) output = docker_utils.run_docker_airbyte_command(["docker", "run", "image", "spec"]) assert output.stderr == "warning: retrying\n" + + +def test_docker_spec_test_fails_on_a_structural_lint(monkeypatch: pytest.MonkeyPatch) -> None: + spec_message = _spec_message({"a": {"type": "string", "display_type": "dropdown"}}) + _fake_docker(monkeypatch, {"spec": _output(spec_message)}) + with pytest.raises(AssertionError, match="/properties/a/display_type"): + _DockerSuite().test_docker_image_build_and_spec( + connector_image_override="source-test:dev", + connector_base_image_override=None, + ) + + +def test_docker_spec_test_warns_on_an_advisory_lint(monkeypatch: pytest.MonkeyPatch) -> None: + from airbyte_cdk.test.standard_tests import SpecLintWarning + + _fake_docker(monkeypatch, {"spec": _output(_spec_message({"a": {"type": "object"}}))}) + with pytest.warns(SpecLintWarning, match="/properties/a"): + _DockerSuite().test_docker_image_build_and_spec( + connector_image_override="source-test:dev", + connector_base_image_override=None, + ) + + +class _EmptyObjectSpecSource(_LeakySource): + def spec(self, logger: logging.Logger) -> ConnectorSpecification: + return ConnectorSpecification(connectionSpecification=_spec({"a": {"type": "object"}})) + + +def test_in_process_spec_test_warns_on_an_advisory_lint(monkeypatch: pytest.MonkeyPatch) -> None: + from airbyte_cdk.test.standard_tests import SpecLintWarning + + monkeypatch.setattr(_InProcessSuite, "connector", _EmptyObjectSpecSource) + with pytest.warns(SpecLintWarning, match="/properties/a"): + _InProcessSuite().test_spec() diff --git a/unit_tests/test/test_spec_structural_lints.py b/unit_tests/test/test_spec_structural_lints.py new file mode 100644 index 000000000..443489843 --- /dev/null +++ b/unit_tests/test/test_spec_structural_lints.py @@ -0,0 +1,572 @@ +# Copyright (c) 2026 Airbyte, Inc., all rights reserved. +"""Unit tests for the structural, presentational and OAuth spec lints in `_spec_lint`.""" + +import warnings +from typing import Any, Callable, Mapping + +import pytest + +from airbyte_cdk.models import ConnectorSpecification +from airbyte_cdk.test.standard_tests import SpecLintWarning +from airbyte_cdk.test.standard_tests._spec_lint import ( + SPEC_LINTS, + SpecLint, + assert_spec_is_valid, + find_additional_properties_errors, + find_array_type_errors, + find_display_type_errors, + find_duplicate_order_errors, + find_empty_object_errors, + find_enum_usage_errors, + find_forbidden_keyword_errors, + find_invalid_schema_errors, + find_nested_group_errors, + find_oauth_parameter_errors, + find_oneof_usage_errors, + find_property_type_errors, + find_spec_problems, + find_unresolved_ref_errors, + format_spec_problems, +) + +Lint = Callable[[Mapping[str, Any]], list[str]] + + +def _object(properties: dict[str, Any], **keywords: Any) -> dict[str, Any]: + return {"type": "object", "properties": properties, **keywords} + + +def _variant( + discriminator: dict[str, Any], name: str = "auth_type", **properties: Any +) -> dict[str, Any]: + return _object({name: discriminator, **properties}) + + +def _with_oneof(*variants: Any, **keywords: Any) -> dict[str, Any]: + return _object({"credentials": {"type": "object", "oneOf": list(variants), **keywords}}) + + +def _assert_errors(lint: Lint, schema: Mapping[str, Any], expected: list[str]) -> None: + """Assert one error per expected pointer, each naming its pointer, in order.""" + errors = lint(schema) + assert len(errors) == len(expected), errors + for error, pointer in zip(errors, expected): + assert error.startswith(f"`{pointer}`"), (error, pointer) + + +OAUTH_TYPE = {"type": "string", "const": "oauth"} +TOKEN_TYPE = {"type": "string", "const": "token"} + + +@pytest.mark.parametrize( + "schema, expected", + [ + pytest.param(_with_oneof(_variant(OAUTH_TYPE), _variant(TOKEN_TYPE)), [], id="const"), + pytest.param( + _with_oneof( + _variant({**OAUTH_TYPE, "default": "oauth", "enum": ["oauth"]}), + _variant(TOKEN_TYPE), + ), + [], + id="const_with_matching_default_and_enum", + ), + pytest.param( + _with_oneof( + _variant({"type": "string", "enum": ["oauth"], "default": "oauth"}), + _variant({"type": "string", "enum": ["token"], "default": "token"}), + ), + [], + id="single_item_enum_with_default", + ), + pytest.param( + _with_oneof( + _variant(OAUTH_TYPE, codec={"type": "string", "enum": ["gzip", "none"]}), + _variant(TOKEN_TYPE, codec={"type": "string", "enum": ["gzip", "none"]}), + ), + [], + id="shared_multi_item_enum_is_not_a_candidate", + ), + pytest.param( + _with_oneof( + _variant(OAUTH_TYPE, mode={"type": "string", "enum": ["a"], "default": "a"}), + _variant(TOKEN_TYPE, mode={"type": "string", "enum": ["b"], "default": "b"}), + ), + [], + id="one_const_candidate_wins_over_enum_candidates", + ), + pytest.param( + _object({"credentials": {"oneOf": [_variant(OAUTH_TYPE), _variant(TOKEN_TYPE)]}}), + ["/properties/credentials/oneOf"], + id="not_an_object", + ), + pytest.param( + _with_oneof(_variant(OAUTH_TYPE), {"type": "object"}), + ["/properties/credentials/oneOf/1"], + id="variant_without_properties", + ), + pytest.param( + _with_oneof(_variant(OAUTH_TYPE), _variant(TOKEN_TYPE, name="method")), + ["/properties/credentials/oneOf"], + id="no_shared_discriminator", + ), + pytest.param( + _with_oneof( + _variant(OAUTH_TYPE, kind={"const": "a"}), + _variant(TOKEN_TYPE, kind={"const": "b"}), + ), + ["/properties/credentials/oneOf"], + id="two_const_discriminators", + ), + pytest.param( + _with_oneof( + _variant({"type": "string", "enum": ["oauth"], "default": "oauth"}), + _variant({"type": "string", "enum": ["token"], "default": "token"}), + _variant({"type": "string", "enum": ["key"], "default": "key"}, name="x"), + ), + ["/properties/credentials/oneOf"], + id="discriminator_missing_from_a_variant", + ), + pytest.param( + _with_oneof(_variant({**OAUTH_TYPE, "default": "token"}), _variant(TOKEN_TYPE)), + ["/properties/credentials/oneOf/0/properties/auth_type"], + id="default_differs_from_const", + ), + pytest.param( + _with_oneof(_variant({**OAUTH_TYPE, "enum": ["token"]}), _variant(TOKEN_TYPE)), + ["/properties/credentials/oneOf/0/properties/auth_type"], + id="enum_differs_from_const", + ), + pytest.param( + _with_oneof( + _variant({"type": "string", "enum": ["oauth"]}), + _variant({"type": "string", "enum": ["token"], "default": "token"}), + ), + ["/properties/credentials/oneOf/0/properties/auth_type"], + id="enum_discriminator_without_default", + ), + pytest.param( + _with_oneof(_variant(OAUTH_TYPE), _variant(OAUTH_TYPE)), + ["/properties/credentials/oneOf"], + id="duplicate_discriminator_value", + ), + ], +) +def test_find_oneof_usage_errors(schema: dict[str, Any], expected: list[str]) -> None: + _assert_errors(find_oneof_usage_errors, schema, expected) + + +@pytest.mark.parametrize( + "lint, schema, expected", + [ + # enum usage + pytest.param( + find_enum_usage_errors, + _object({"a": {"enum": ["x", "y", 1, {"k": 1}]}}), + [], + id="enum_ok", + ), + pytest.param( + find_enum_usage_errors, + _object({"a": {"enum": ["x", "y", "x", "x"]}}), + ["/properties/a/enum", "/properties/a/enum"], + id="enum_duplicates", + ), + # forbidden keywords + pytest.param( + find_forbidden_keyword_errors, + _object({"not": {"type": "string"}, "if": {"type": "string"}}), + [], + id="forbidden_keyword_as_property_name", + ), + pytest.param( + find_forbidden_keyword_errors, + _object( + { + "a": {"allOf": [{"type": "string"}]}, + "b": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + }, + patternProperties={"^x": {"type": "string"}}, + ), + ["/patternProperties", "/properties/a/allOf", "/properties/b/anyOf"], + id="forbidden_keywords", + ), + # array items + pytest.param( + find_array_type_errors, + _object( + { + "a": {"type": "array", "items": {"type": "string"}}, + "b": {"type": "array", "items": {"enum": ["x", "y"]}}, + "c": {"type": "array", "items": _object({"x": {"type": "string"}})}, + "d": {"type": "array"}, + } + ), + [], + id="array_ok", + ), + pytest.param( + find_array_type_errors, + _object( + { + "a": {"type": "array", "items": [{"type": "string"}]}, + "b": {"type": "array", "items": {"type": "boolean"}}, + "c": {"type": "array", "items": {"type": "array"}}, + } + ), + ["/properties/a/items", "/properties/b/items", "/properties/c/items"], + id="array_bad_items", + ), + # object not empty + pytest.param(find_empty_object_errors, {"type": "object"}, [], id="empty_root_ok"), + pytest.param( + find_empty_object_errors, + _object( + { + "a": _object({"x": {"type": "string"}}), + "b": {"type": "object", "oneOf": [_object({"x": {"const": "y"}})]}, + } + ), + [], + id="objects_ok", + ), + pytest.param( + find_empty_object_errors, + _object( + { + "a": {"type": "object"}, + "b": {"type": "object", "properties": {}}, + "c": {"type": "array", "items": {"type": "object"}}, + } + ), + ["/properties/a", "/properties/b", "/properties/c/items"], + id="empty_objects", + ), + # property types + pytest.param( + find_property_type_errors, + _object( + { + "a": {"type": "string"}, + "b": {"type": ["string"]}, + "c": {"type": ["integer", "null"]}, + } + ), + [], + id="property_types_ok", + ), + pytest.param( + find_property_type_errors, + _object( + { + "a": {"type": ["null", "string"]}, + "b": {"type": ["string", "integer"]}, + "c": {"type": ["string", "integer", "null"]}, + } + ), + ["/properties/a/type", "/properties/b/type", "/properties/c/type"], + id="property_types_bad", + ), + # additionalProperties + pytest.param( + find_additional_properties_errors, + _object({"a": _object({}, additionalProperties=True)}, additionalProperties=True), + [], + id="additional_properties_true", + ), + pytest.param( + find_additional_properties_errors, + _object( + {"a": _object({}, additionalProperties={"type": "string"})}, + additionalProperties=False, + ), + ["/additionalProperties", "/properties/a/additionalProperties"], + id="additional_properties_not_true", + ), + # display_type + pytest.param( + find_display_type_errors, + _with_oneof(_variant(OAUTH_TYPE), display_type="radio"), + [], + id="display_type_ok", + ), + pytest.param( + find_display_type_errors, + _object( + { + "a": {"type": "string", "display_type": "dropdown"}, + "b": {"type": "object", "oneOf": [], "display_type": "list"}, + } + ), + ["/properties/a/display_type", "/properties/b/display_type"], + id="display_type_bad", + ), + # duplicate order + pytest.param( + find_duplicate_order_errors, + _object( + { + "a": {"type": "string", "order": 0}, + "b": {"type": "string", "order": 1}, + "c": {"type": "string", "order": 0, "group": "advanced"}, + "d": _object( + {"x": {"type": "string", "order": 0}, "y": {"type": "string", "order": 1}} + ), + } + ), + [], + id="order_ok", + ), + pytest.param( + find_duplicate_order_errors, + _object( + { + "a": {"type": "string", "order": 0}, + "b": {"type": "string", "order": 0}, + "c": {"type": "string", "order": 1, "group": "g"}, + "d": {"type": "string", "order": 1, "group": "g"}, + } + ), + ["/properties/b/order", "/properties/d/order"], + id="order_duplicates", + ), + # nested group + pytest.param( + find_nested_group_errors, + _object({"a": {"type": "string", "group": "auth"}}), + [], + id="group_top_level", + ), + pytest.param( + find_nested_group_errors, + _object({"a": _object({"b": {"type": "string", "group": "auth"}})}), + ["/properties/a/properties/b/group"], + id="group_nested", + ), + # valid JSON Schema + pytest.param( + find_invalid_schema_errors, + _object({"a": {"type": "string", "minLength": 1}}, required=["a"]), + [], + id="valid_schema", + ), + pytest.param( + find_invalid_schema_errors, + _object({"a": {"type": "text"}}, required="a"), + ["/properties/a/type", "/required"], + id="invalid_schema", + ), + ], +) +def test_structural_lints(lint: Lint, schema: dict[str, Any], expected: list[str]) -> None: + errors = lint(schema) + assert len(errors) == len(expected), errors + for error, pointer in zip(sorted(errors), sorted(expected)): + assert error.startswith(f"`{pointer}"), (error, pointer) + + +def test_nested_group_error_names_the_group_keyword() -> None: + errors = find_nested_group_errors(_object({"a": _object({"b": {"group": "g"}})})) + assert errors == [ + "`/properties/a/properties/b/group` is set below the top level; only top-level " + "properties can be grouped." + ] + + +def test_lints_ignore_values_that_look_like_schemas() -> None: + schema = _object( + {"a": {"type": "string", "default": {"type": "object"}, "examples": [{"allOf": []}]}} + ) + for lint in (find_empty_object_errors, find_forbidden_keyword_errors): + assert lint(schema) == [] + + +@pytest.mark.parametrize( + "spec, expected", + [ + pytest.param({"connectionSpecification": _object({"a": {"type": "string"}})}, [], id="ok"), + pytest.param( + { + "connectionSpecification": _object({"a": {"$ref": "#/definitions/a"}}), + "advanced_auth": {"oauth_config_specification": {"x": {"$ref": "#/y"}}}, + }, + [ + "/connectionSpecification/properties/a/$ref", + "/advanced_auth/oauth_config_specification/x/$ref", + ], + id="refs", + ), + ], +) +def test_find_unresolved_ref_errors(spec: dict[str, Any], expected: list[str]) -> None: + _assert_errors(find_unresolved_ref_errors, spec, expected) + + +def _oauth_spec(predicate_key: list[str], output_path: list[str]) -> dict[str, Any]: + return { + "connectionSpecification": _object( + { + "credentials": { + "type": "object", + "oneOf": [ + _variant( + OAUTH_TYPE, + client_id={"type": "string", "airbyte_secret": True}, + refresh_token={"type": "string", "airbyte_secret": True}, + ), + _variant(TOKEN_TYPE, api_token={"type": "string", "airbyte_secret": True}), + ], + }, + "accounts": { + "type": "array", + "items": _object({"id": {"type": "string"}}), + }, + } + ), + "advanced_auth": { + "auth_flow_type": "oauth2.0", + "predicate_key": predicate_key, + "predicate_value": "oauth", + "oauth_config_specification": { + "complete_oauth_output_specification": { + "type": "object", + "properties": { + "refresh_token": { + "type": "string", + "path_in_connector_config": output_path, + } + }, + }, + "complete_oauth_server_output_specification": { + "type": "object", + "properties": { + "client_id": { + "type": "string", + "path_in_connector_config": ["credentials", "client_id"], + }, + "account": { + "type": "string", + "path_in_connector_config": ["accounts", "[]", "id"], + }, + }, + }, + }, + }, + } + + +def test_find_oauth_parameter_errors_accepts_paths_through_oneof_and_arrays() -> None: + spec = _oauth_spec(["credentials", "auth_type"], ["credentials", "refresh_token"]) + assert find_oauth_parameter_errors(spec) == [] + + +def test_find_oauth_parameter_errors_reports_each_missing_path() -> None: + spec = _oauth_spec(["credentials", "method"], ["refresh_token"]) + assert find_oauth_parameter_errors(spec) == [ + "`/advanced_auth/predicate_key` names the config path `/credentials/method`, which is " + "not a property of `connectionSpecification`.", + "`/advanced_auth/oauth_config_specification/complete_oauth_output_specification/" + "properties/refresh_token/path_in_connector_config` names the config path " + "`/refresh_token`, which is not a property of `connectionSpecification`.", + ] + + +def test_find_oauth_parameter_errors_without_advanced_auth() -> None: + assert find_oauth_parameter_errors({"connectionSpecification": _object({})}) == [] + + +def test_pointer_escapes_slash_and_tilde() -> None: + errors = find_additional_properties_errors( + _object({"a/b": _object({}, additionalProperties=False), "c~d": {"type": "string"}}) + ) + assert errors[0].startswith("`/properties/a~1b/additionalProperties`") + + +def test_spec_lints_have_unique_names() -> None: + names = [lint.name for lint in SPEC_LINTS] + assert len(names) == len(set(names)) + + +def test_advisory_lints() -> None: + assert {lint.name for lint in SPEC_LINTS if not lint.enforced} == { + "forbidden keywords", + "object not empty", + } + + +def test_find_spec_problems_reports_a_crashing_lint_without_raising() -> None: + def crash(spec: Mapping[str, Any]) -> list[str]: + raise KeyError("sk_live_should_not_be_printed") + + lint = SpecLint("crash", "Never crashes.", crash, enforced=False) + problems = find_spec_problems({"connectionSpecification": _object({})}, lints=[lint]) + assert problems == {lint: ["The lint could not run: KeyError."]} + + +def test_find_spec_problems_on_a_clean_spec() -> None: + spec = { + "connectionSpecification": _object( + {"api_key": {"type": "string", "airbyte_secret": True, "order": 0}}, + additionalProperties=True, + ) + } + assert find_spec_problems(spec) == {} + + +def test_format_spec_problems_groups_by_lint() -> None: + first = SpecLint("first", "Rule one.", lambda spec: []) + second = SpecLint("second", "Rule two.", lambda spec: []) + text = format_spec_problems({first: ["a", "b"], second: ["c"]}) + assert text == "[first] Rule one.\n - a\n - b\n[second] Rule two.\n - c" + + +def _connector_spec(properties: dict[str, Any], **keywords: Any) -> ConnectorSpecification: + return ConnectorSpecification(connectionSpecification=_object(properties, **keywords)) + + +def test_assert_spec_is_valid_fails_on_an_enforced_lint_and_names_it() -> None: + spec = _connector_spec({"a": {"type": "string"}}, additionalProperties=False) + with pytest.raises(AssertionError) as error: + assert_spec_is_valid(spec, connector_name="source-test") + message = str(error.value) + assert "[additionalProperties is true]" in message + assert "`/additionalProperties`" in message + assert "connector-specification-reference" in message + + +def test_assert_spec_is_valid_warns_on_an_advisory_lint() -> None: + spec = _connector_spec({"a": {"type": "object"}, "b": {"allOf": [{"type": "string"}]}}) + with pytest.warns(SpecLintWarning) as record: + assert_spec_is_valid(spec, connector_name="source-test") + assert len(record) == 1 + message = str(record[0].message) + assert "[object not empty]" in message and "`/properties/a`" in message + assert "[forbidden keywords]" in message and "`/properties/b/allOf`" in message + + +def test_assert_spec_is_valid_warns_before_it_fails() -> None: + spec = _connector_spec({"a": {"type": "object"}}, additionalProperties=False) + with pytest.warns(SpecLintWarning), pytest.raises(AssertionError) as error: + assert_spec_is_valid(spec, connector_name="source-test") + assert "object not empty" not in str(error.value) + + +def test_assert_spec_is_valid_is_silent_on_a_clean_spec() -> None: + spec = _connector_spec({"a": {"type": "string", "order": 0}}) + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert_spec_is_valid(spec, connector_name="source-test") + + +def test_failure_messages_do_not_print_values() -> None: + marker = "sk_live_value_from_the_spec" + spec = _connector_spec( + { + "a": {"type": "string", "enum": [marker, marker], "display_type": marker}, + "b": {"type": "string", "order": 3, "default": marker}, + "c": {"type": "string", "order": 3, "examples": [marker]}, + }, + additionalProperties={"description": marker}, + ) + with pytest.raises(AssertionError) as error: + assert_spec_is_valid(spec, connector_name="source-test") + assert marker not in str(error.value) From 5e43be1609d34f39a382019eb1202b9bdfa656f8 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 15:55:28 +0300 Subject: [PATCH 2/4] fix(standard-tests): report non-string spec keyword values instead of crashing Three enforced spec lints crashed on keyword values that are not hashable strings: `array items` on an `items.type` list such as `[string, null]`, `display_type` on a list or object value, and `valid JSON Schema` on a non-string `$schema`. `find_spec_problems` turned the crash into "The lint could not run: TypeError." with no pointer, so the author could not find the problem. Each lint now reports the pointer like any other problem, and the schema check reuses `_catalog_checks._meta_validator`, which falls back to draft 7 for a non-string `$schema` (its meta-schema then reports it). The migration note now names the two connectors that still fail an enforced lint (archived destination-timeplus, and source-mssql in airbyte's Dockerfile Tests workflow) instead of claiming every failing connector was fixed. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/standard_tests/_spec_lint.py | 21 +++--- cdk-migrations.md | 7 +- unit_tests/test/test_spec_structural_lints.py | 67 +++++++++++++++++++ 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/_spec_lint.py b/airbyte_cdk/test/standard_tests/_spec_lint.py index 09214111d..378951f4c 100644 --- a/airbyte_cdk/test/standard_tests/_spec_lint.py +++ b/airbyte_cdk/test/standard_tests/_spec_lint.py @@ -16,8 +16,6 @@ from typing import Any from urllib.parse import quote -import jsonschema - from airbyte_cdk.models import ( AirbyteMessage, AirbyteMessageSerializer, @@ -26,7 +24,7 @@ Type, ) from airbyte_cdk.test.entrypoint_wrapper import EntrypointOutput -from airbyte_cdk.test.standard_tests._catalog_checks import _iter_subschemas +from airbyte_cdk.test.standard_tests._catalog_checks import _iter_subschemas, _meta_validator SECRET_PROPERTY_NAMES = frozenset( { @@ -582,7 +580,9 @@ def find_array_type_errors(connection_specification: Mapping[str, Any]) -> list[ where = _spec_pointer((*path, "items")) if not isinstance(items, Mapping): errors.append(f"{where} must be a single schema, not a list of item schemas.") - elif items.get("type") not in ARRAY_ITEM_TYPES and "enum" not in items: + elif "enum" in items: + continue + elif not isinstance(items.get("type"), str) or items["type"] not in ARRAY_ITEM_TYPES: errors.append( f"{where} must be of type `object`, `string`, `number` or `integer`, or set " "an `enum`." @@ -679,7 +679,8 @@ def find_display_type_errors(connection_specification: Mapping[str, Any]) -> lis where = _spec_pointer((*path, "display_type")) if "oneOf" not in schema: errors.append(f"{where} is set on a field without `oneOf` variants.") - if schema["display_type"] not in DISPLAY_TYPES: + display_type = schema["display_type"] + if not isinstance(display_type, str) or display_type not in DISPLAY_TYPES: errors.append(f"{where} must be `dropdown` or `radio`.") return errors @@ -799,18 +800,14 @@ def find_invalid_schema_errors(connection_specification: Mapping[str, Any]) -> l """Return one error per place where the spec is not a valid JSON Schema. The spec is checked against the meta-schema of the draft it declares in `$schema`, draft 7 - by default. CAT only found this through `test_config_match_spec`, when a scenario config + by default, and draft 7 when `$schema` is not a string (its meta-schema then reports + `$schema`). CAT only found this through `test_config_match_spec`, when a scenario config was validated against an invalid spec. """ - validator_class = jsonschema.validators.validator_for( - connection_specification, default=jsonschema.Draft7Validator - ) return [ f"{_spec_pointer(map(str, error.path))} is not valid JSON Schema: it fails the " f"`{error.validator}` rule of the meta-schema." - for error in validator_class(validator_class.META_SCHEMA).iter_errors( - connection_specification - ) + for error in _meta_validator(connection_specification).iter_errors(connection_specification) ] diff --git a/cdk-migrations.md b/cdk-migrations.md index e348cda6c..29cabf20d 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -2,7 +2,10 @@ ## Upgrading to the next minor release (unreleased) -This release adds structural lints to the spec tests (`test_spec` and `test_docker_image_build_and_spec`), ported from the spec checks of the Connector Acceptance Tests (CAT). They run on the spec that `spec` emits, need no credentials, and report every problem with the JSON pointer of the place in the spec, never a value. The release is a minor one, since only the test harness changes. Every connector in the monorepo that failed an enforced lint was fixed before this release. +This release adds structural lints to the spec tests (`test_spec` and `test_docker_image_build_and_spec`), ported from the spec checks of the Connector Acceptance Tests (CAT). They run on the spec that `spec` emits, need no credentials, and report every problem with the JSON pointer of the place in the spec, never a value. The release is a minor one, since only the test harness changes. The monorepo connectors that failed an enforced lint were fixed before this release, with two known exceptions, both failing `additionalProperties is true`: + +- `destination-timeplus` is archived and disabled in both registries, so nobody ships it. Its tests fail only when a pull request edits it. +- `source-mssql` (Java) emits a map property whose `additionalProperties` is a schema. It fails the `airbyte-cdk image test` step of airbyte's Dockerfile Tests workflow, which runs only when a pull request changes the Java connector Dockerfiles. ### Enforced spec lints @@ -15,7 +18,7 @@ The spec tests now fail when the emitted spec breaks one of these rules. Each fa | additionalProperties is true | `additionalProperties` is `true` wherever it is set. With `false` or a schema, removing a property later invalidates every saved config that sets it. | Set it to `true`, or remove it. Relaxing `false` to `true` is backward compatible. | | oneOf usage | A schema with `oneOf` is `type: object`, every variant has `properties`, and the variants share exactly one discriminator: a property that sets `const` in every variant or, failing that, a single-item `enum` with that item as `default`. A `const` discriminator may set `default` and `enum` only to its `const`, and no two variants share a discriminator value. | Add a shared `const` property, such as `auth_type`, to each variant. | | enum usage | An `enum` does not list the same value twice. | Remove the duplicate. | -| array items | An array's `items` is one schema, of type `object`, `string`, `number` or `integer`, or with an `enum`. | Change the item type. | +| array items | An array's `items` is one schema, of type `object`, `string`, `number` or `integer` (a single type, not a list such as `[string, null]`), or with an `enum`. | Change the item type. | | property types | A property whose `type` is a list holds one type, or one type followed by `null`. The setup form renders the first type only. | Reorder `[null, string]` to `[string, null]`. | | display_type | `display_type` is set only on a field with `oneOf` variants, to `dropdown` or `radio`. | Remove it or fix its value. | | duplicate order | No two properties of one object (and one `group`) share an `order`. | Give each property a unique `order`. `order` is an annotation, so changing it is backward compatible. | diff --git a/unit_tests/test/test_spec_structural_lints.py b/unit_tests/test/test_spec_structural_lints.py index 443489843..244750960 100644 --- a/unit_tests/test/test_spec_structural_lints.py +++ b/unit_tests/test/test_spec_structural_lints.py @@ -355,6 +355,49 @@ def test_find_oneof_usage_errors(schema: dict[str, Any], expected: list[str]) -> ["/properties/a/type", "/required"], id="invalid_schema", ), + # Keyword values that are not strings are reported at their pointer, never crash. + pytest.param( + find_array_type_errors, + _object( + { + "a": {"type": "array", "items": {"type": ["string", "null"]}}, + "b": {"type": "array", "items": {"type": {"x": 1}}}, + } + ), + ["/properties/a/items", "/properties/b/items"], + id="array_items_type_not_a_string", + ), + pytest.param( + find_display_type_errors, + _object( + { + "a": { + "type": "object", + "oneOf": [_variant(OAUTH_TYPE)], + "display_type": ["radio"], + }, + "b": { + "type": "object", + "oneOf": [_variant(OAUTH_TYPE)], + "display_type": {"x": 1}, + }, + } + ), + ["/properties/a/display_type", "/properties/b/display_type"], + id="display_type_not_a_string", + ), + pytest.param( + find_invalid_schema_errors, + {**_object({"a": {"type": "string"}}), "$schema": ["draft-07"]}, + ["/$schema"], + id="schema_keyword_not_a_string", + ), + pytest.param( + find_invalid_schema_errors, + {**_object({"a": {"type": "string"}}), "$schema": {"x": 1}}, + ["/$schema"], + id="schema_keyword_an_object", + ), ], ) def test_structural_lints(lint: Lint, schema: dict[str, Any], expected: list[str]) -> None: @@ -502,6 +545,30 @@ def crash(spec: Mapping[str, Any]) -> list[str]: assert problems == {lint: ["The lint could not run: KeyError."]} +@pytest.mark.parametrize( + "connection_specification", + [ + pytest.param( + _object({"a": {"type": "array", "items": {"type": ["string", "null"]}}}), + id="array_items_type_list", + ), + pytest.param( + _object({"a": {"type": "object", "oneOf": [], "display_type": ["radio"]}}), + id="display_type_list", + ), + pytest.param({"type": "object", "$schema": ["draft-07"]}, id="schema_keyword_list"), + ], +) +def test_find_spec_problems_names_the_pointer_of_a_non_string_keyword_value( + connection_specification: dict[str, Any], +) -> None: + problems = find_spec_problems({"connectionSpecification": connection_specification}) + messages = [message for found in problems.values() for message in found] + assert messages + assert not any("could not run" in message for message in messages), messages + assert all(message.startswith("`/") for message in messages), messages + + def test_find_spec_problems_on_a_clean_spec() -> None: spec = { "connectionSpecification": _object( From e2b6d92c75835e2f0e620e7a284c61b657a3ecd5 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 16:12:07 +0300 Subject: [PATCH 3/4] docs(standard-tests): scope the spec lint migration note to connectors that run the spec tests The note said the monorepo connectors that failed an enforced lint were fixed "with two known exceptions". source-tplcentral also fails until its separate, non-blocking spec fix merges, because its legacy `tests:` config keeps the lints from running at all. Scope the sentence to connectors whose CI runs the CDK spec tests and list source-tplcentral as a third exception. Co-Authored-By: Claude Opus 5.5 --- cdk-migrations.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cdk-migrations.md b/cdk-migrations.md index 29cabf20d..e2602660a 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -2,10 +2,11 @@ ## Upgrading to the next minor release (unreleased) -This release adds structural lints to the spec tests (`test_spec` and `test_docker_image_build_and_spec`), ported from the spec checks of the Connector Acceptance Tests (CAT). They run on the spec that `spec` emits, need no credentials, and report every problem with the JSON pointer of the place in the spec, never a value. The release is a minor one, since only the test harness changes. The monorepo connectors that failed an enforced lint were fixed before this release, with two known exceptions, both failing `additionalProperties is true`: +This release adds structural lints to the spec tests (`test_spec` and `test_docker_image_build_and_spec`), ported from the spec checks of the Connector Acceptance Tests (CAT). They run on the spec that `spec` emits, need no credentials, and report every problem with the JSON pointer of the place in the spec, never a value. The release is a minor one, since only the test harness changes. The monorepo connectors whose CI runs the CDK spec tests and that failed an enforced lint were fixed before this release, with these known exceptions: -- `destination-timeplus` is archived and disabled in both registries, so nobody ships it. Its tests fail only when a pull request edits it. -- `source-mssql` (Java) emits a map property whose `additionalProperties` is a schema. It fails the `airbyte-cdk image test` step of airbyte's Dockerfile Tests workflow, which runs only when a pull request changes the Java connector Dockerfiles. +- `destination-timeplus` fails `additionalProperties is true`. It is archived and disabled in both registries, so nobody ships it. Its tests fail only when a pull request edits it. +- `source-mssql` (Java) fails `additionalProperties is true`: it emits a map property whose `additionalProperties` is a schema. It fails the `airbyte-cdk image test` step of airbyte's Dockerfile Tests workflow, which runs only when a pull request changes the Java connector Dockerfiles. +- `source-tplcentral` fails `additionalProperties is true` until its spec fix merges. Its legacy top-level `tests:` layout in `acceptance-test-config.yml` fails test collection, so the lints do not run on it until that config is converted. ### Enforced spec lints From 01f7ebb1e07c6272042a3ec762cfdb06e6c72fa0 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Mon, 19 Oct 2026 15:56:09 +0300 Subject: [PATCH 4/4] docs(standard-tests): keep null when rewriting a pydantic Optional anyOf Dropping the null branch of anyOf: [X, null] rejects saved configs that set the field to null. Rewrite it as X with type: [, "null"] instead, which accepts exactly the same configs. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/standard_tests/_spec_lint.py | 5 +++-- cdk-migrations.md | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/airbyte_cdk/test/standard_tests/_spec_lint.py b/airbyte_cdk/test/standard_tests/_spec_lint.py index 378951f4c..413475af3 100644 --- a/airbyte_cdk/test/standard_tests/_spec_lint.py +++ b/airbyte_cdk/test/standard_tests/_spec_lint.py @@ -905,8 +905,9 @@ def run(spec: Mapping[str, Any]) -> list[str]: ), # Not enforced yet. `source-facebook-marketing` (a single-item `allOf` that pydantic emits # for an enum field) and `destination-ragie` (pydantic-v2 `Optional` fields as `anyOf`) - # build their specs in code. Inlining those schemas accepts exactly the same configs, but the - # spec backward-compatibility check reports it as breaking. + # build their specs in code. Inlining the `allOf`, or rewriting `anyOf: [X, null]` as `X` with + # `type: [, "null"]`, accepts exactly the same configs, but the spec + # backward-compatibility check reports it as breaking. SpecLint( "forbidden keywords", "The connector setup form cannot render these JSON Schema keywords.", diff --git a/cdk-migrations.md b/cdk-migrations.md index e2602660a..e51cc171c 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -30,7 +30,7 @@ The spec tests now fail when the emitted spec breaks one of these rules. Each fa These lints are reported as a `SpecLintWarning` and do not fail the test yet; a later release will enforce them: -- **forbidden keywords**: the spec uses a keyword the setup form cannot render: `not`, `anyOf`, `allOf`, `patternProperties`, `prefixItems`, `if`, `then`, `else`, `dependentSchemas` or `dependentRequired`. Pydantic emits `allOf` for a field typed as an enum and `anyOf` for an `Optional` field (pydantic v2); inline the referenced schema, and for an optional field drop the `null` branch and the field from `required`. +- **forbidden keywords**: the spec uses a keyword the setup form cannot render: `not`, `anyOf`, `allOf`, `patternProperties`, `prefixItems`, `if`, `then`, `else`, `dependentSchemas` or `dependentRequired`. Pydantic emits `allOf` for a field typed as an enum and `anyOf` for an `Optional` field (pydantic v2); inline the referenced schema. For an `Optional` field, replace `anyOf: [X, {type: null}]` with `X`'s keywords on the property and a `type` that lists `X`'s type followed by `null`, for example `type: [string, "null"]`. Keep the `null`: dropping it rejects every saved config that sets the field to `null`. - **object not empty**: a nested `type: object` declares no `properties` and no `oneOf`, so the setup form shows nothing for it. Declare its properties, or remove it. To make the warnings fail the test now, run `airbyte-cdk connector test --pytest-arg=-Werror::airbyte_cdk.test.standard_tests.SpecLintWarning`, or pass `-W error::airbyte_cdk.test.standard_tests.SpecLintWarning` to `pytest`.