diff --git a/CHANGELOG.md b/CHANGELOG.md index eee10d00..54785486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `iceberg_catalog_move_blocked` and prints the `tofu state rm` commands that release them, instead of planning to destroy them: destroying a Glue database deletes every table in it. The resources stay in AWS; run the commands, then apply again. +- The same guard covers Snowflake. An Iceberg expose on `platform: snowflake` with + `catalog: lakekeeper`, `bigquery`, the `iceberg-rest` spelling, `hive`, `jdbc`, + `hadoop` or `dynamodb` got a Snowflake EXTERNAL VOLUME from 0.19.0 and earlier and no + longer does (`rest`, `iceberg_rest`, `polaris`, `unity`, `nessie` and `glue` never + had one). `fluid apply` stops with `iceberg_catalog_move_blocked` and the `tofu state + rm` command for the volume instead of planning to drop it. +- A GCP Iceberg expose that a streaming sink writes to must now name its catalog in + `binding.location.catalog`. Without one, the sink wrote through whatever catalog + reached the worker (REST by default, Glue for `sink.catalog: glue` or a hand-written + `catalog-impl`) while dbt-bigquery and the GCP IaC created a BigLake table, so one + table lived in two catalogs; `fluid validate` and the runners now refuse it unless the + sink writes BigLake. Set `catalog: bigquery`, or the REST kind your catalog is. A + hand-written sink config must then select that same catalog. ### Changed @@ -47,6 +60,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Command output keeps its square brackets.** Rich read text in brackets as console + markup and dropped it, so `fluid validate` printed `exposes declares + governance.lakeFormation` instead of `exposes[orders] declares ...`, bundle findings + lost their `[error]`/`[warning]` severity, and a suggestion such as `pip install + 'data-product-forge[gcp]'` lost its extra. Validation messages, error context and + suggestions now print literally, and a printed `tofu state rm
` command stays + on one line, so a copied line runs with its address. +- **The streaming runners check the sink they push.** The Kafka Connect and embedded + Debezium Server runners ran the Iceberg sink checks only for a config they derived; a + hand-written `sink_connector_config` or `server.sink.config` that `fluid validate` + refuses was still deployed. A hand-written config in a build that declares + `sink.format: iceberg` is now checked the same way, before anything is created. In a + contract with several builds, each runner also reads the build it is running instead + of the first one, which the checks already did. A `sink_connector_config`, + `server.sink.config` or `iceberg_catalog_overrides` whose `type` or `catalog-impl` + selects a different catalog than the expose's is now an error on every platform: the + sink would write one catalog while dbt and the IaC read another. Declare the catalog + the sink writes to in `binding.location.catalog`; a REST endpoint that fronts Glue + (Glue's Iceberg REST endpoint) is `catalog: rest`. +- **`catalog: bigquery` on a Kafka Connect sink warns.** It sets + `iceberg.catalog.type=bigquery`, which the published Apache Iceberg sink (1.9.2) cannot + load; Iceberg adds the type in 1.10. The Nessie and BigQuery runtime warnings follow the + catalog type that actually reaches the worker, so a hand-written config that sets + `type: rest` no longer draws them. +- **A typo in `location.catalog` is reported once, as a typo.** The native AWS planner now + refuses an unknown catalog value as the IaC does (it used to plan the S3 buckets and no + Glue table), without first logging it as a `plan_failed` JSON line, and a typo no longer + also draws a Lake Formation refusal advising to "govern access in the catalog". +- **The Iceberg catalog-move guard no longer blocks a legitimate apply.** It flagged a + Glue database that a since-removed parquet expose had created as a catalog move, and + nothing could get past it. A database is now flagged only when the moved expose's own + Glue table is in state, or when the expose names no table: its database is then all + an earlier release created for it, and the guard used to let its destroy through. On + Snowflake the guard finds the EXTERNAL VOLUME by its contract-derived name, so it also + stops an upgrade that changed `location.warehouse` to a catalog name or dropped + `iam_role_arn` in the same edit. When the guard cannot read state or its probe fails, + it says so at WARNING level instead of DEBUG. + - **A Kafka Connect Iceberg sink on AWS Glue starts.** The derived connector config set both `iceberg.catalog.type` and `iceberg.catalog.catalog-impl`, and Apache Iceberg's `CatalogUtil` refuses that ("both type and catalog-impl are set"), so the sink failed diff --git a/fluid_build/_error_catalog.py b/fluid_build/_error_catalog.py index ebf1e23e..f807b5d7 100644 --- a/fluid_build/_error_catalog.py +++ b/fluid_build/_error_catalog.py @@ -438,6 +438,21 @@ def slug_for(event: str) -> str: ["Pass the product id to roll back: fluid rollback "], None, ), + # ── apply safety gates ───────────────────────────────────────────────── + "iceberg_catalog_move_blocked": ( + [ + ( + "Run the printed `tofu state rm` commands: they release the resources from " + + "this contract's OpenTofu state and touch nothing in the cloud" + ), + "Then re-run fluid apply", + ( + "The released Glue database/table or Snowflake EXTERNAL VOLUME stays in place; " + + "delete it by hand only if nothing else uses it" + ), + ], + "iceberg-catalog-move", + ), } diff --git a/fluid_build/_errors.py b/fluid_build/_errors.py index 9bb3df95..dce8617c 100644 --- a/fluid_build/_errors.py +++ b/fluid_build/_errors.py @@ -81,6 +81,8 @@ "dlq": "advanced/typed-cli-errors.html#pipeline-operations", "replay": "advanced/typed-cli-errors.html#pipeline-operations", "infra#drift": "advanced/typed-cli-errors.html#governance", + # the safety gate's own section, which shows its message and remediation + "iceberg-catalog-move": "cli/apply.html#iceberg-catalog-move-guard", } _DOC_FALLBACK = "advanced/production-troubleshooting.html" diff --git a/fluid_build/build_runners/debezium/runner.py b/fluid_build/build_runners/debezium/runner.py index 270f0d00..295f3fd1 100644 --- a/fluid_build/build_runners/debezium/runner.py +++ b/fluid_build/build_runners/debezium/runner.py @@ -34,7 +34,7 @@ import time from dataclasses import dataclass from pathlib import Path -from typing import Any, ClassVar, Dict, FrozenSet, List, Optional +from typing import Any, ClassVar, Dict, FrozenSet, List, Mapping, Optional from fluid_build.api.runner import ( RunContext, @@ -300,7 +300,22 @@ def _execute(ctx: RunContext, runner: DebeziumRunner) -> RunResult: started_at, t_start = begin_acquisition_run(ctx, runner) - props = ctx.contract.get("builds", [{}])[0].get("properties", {}) + from ..kafka_connect.iceberg_sink_validation import executing_build + + # The build this run executes, the SAME one the Iceberg preflight checks. + # Reading ``builds[0]`` here ran a later build with the first build's + # Debezium settings while the preflight vetted the later build's. + build = executing_build(ctx.contract, ctx.build_id) + if build is None: + return _failed( + ctx, + started_at, + t_start, + f"debezium: the run context's build {ctx.build_id!r} is not in the " + "contract's builds, so its properties cannot be read; run a build the " + "contract declares", + ) + props = build.get("properties") or {} dbz_props = props.get("debezium", {}) or {} deployment = dbz_props.get("deployment", {}) or {} mode = deployment.get("mode", "bring-your-own") @@ -316,7 +331,7 @@ def _execute(ctx: RunContext, runner: DebeziumRunner) -> RunResult: ) if mode == "embedded": return _execute_debezium_server( - ctx, deployment, dbz_props, snapshot_mode, started_at, t_start + ctx, build, deployment, dbz_props, snapshot_mode, started_at, t_start ) return _failed(ctx, started_at, t_start, f"debezium: unknown deployment.mode '{mode}'") @@ -477,6 +492,7 @@ def _escape_properties_value(value: str) -> str: def _execute_debezium_server( ctx: RunContext, + build: Mapping[str, Any], deployment: Dict[str, Any], dbz_props: Dict[str, Any], snapshot_mode: str, @@ -509,28 +525,34 @@ def _execute_debezium_server( # Iceberg sink: derive the bare-key config from the contract's iceberg expose # binding so the embedded server lands in the SAME table the static Glue - # table / KC sink resolve to (RFC zero-drift spine). Mirrors the KC gate: - # derivation is OFF whenever a hand-written ``config`` block is present, so - # those contracts stay byte-for-byte identical (an explicit empty ``config: - # {}`` counts as present, matching the KC ``sink_connector_config`` default); + # table / KC sink resolve to (RFC zero-drift spine). Gated by + # ``iceberg_sink_plan``, the one gate `fluid validate` and the KC runner + # read too: derivation is OFF whenever a hand-written ``config`` block is + # present, so nothing is derived for it (an explicit empty ``config: {}`` + # counts as present, matching the KC ``sink_connector_config`` default); # opt back in with ``server.sink.iceberg_sink_enabled: true``. Derived keys # go UNDER the hand-written ones, so an operator key always wins. + from ..kafka_connect.iceberg_sink_validation import ( + iceberg_sink_plan, + iceberg_sink_preflight, + ) + sink_config = handwritten - iceberg_on = sink_block.get("iceberg_sink_enabled", "config" not in sink_block) - if sink_type == "iceberg" and iceberg_on: + sink_plan = iceberg_sink_plan(build) + if sink_plan is not None: + # Fail closed BEFORE application.properties is written, for a derived + # AND a hand-written Iceberg sink: the same checks `fluid validate` + # runs, so a sink it rejects never boots. + preflight_error = iceberg_sink_preflight(ctx.contract, ctx.build_id, log=LOG) + if preflight_error: + return _failed(ctx, started_at, t_start, preflight_error) + if sink_plan is not None and sink_plan.derives: from ...providers._iceberg_catalog import ( find_iceberg_expose_binding, resolve_iceberg_catalog, ) - from ..kafka_connect.iceberg_sink_validation import iceberg_sink_preflight from .iceberg_sink import emit_debezium_iceberg_sink_config - # Fail closed BEFORE application.properties is written: the same - # checks `fluid validate` runs, so a sink it rejects never boots. - preflight_error = iceberg_sink_preflight(ctx.contract, ctx.build_id, log=LOG) - if preflight_error: - return _failed(ctx, started_at, t_start, preflight_error) - binding = find_iceberg_expose_binding(ctx.contract) if binding is not None: resolved = resolve_iceberg_catalog( diff --git a/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py b/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py index 654f1cc8..4aff656f 100644 --- a/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py +++ b/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py @@ -20,10 +20,13 @@ catalog traps below. It is a pure function returning (errors, warnings) — the validate stage routes errors to its collector and surfaces warnings, mirroring product_types.py. The Kafka-Connect and Debezium-Server runners run the SAME -checks through :func:`iceberg_sink_preflight` right before they derive a sink, -so a contract ``fluid validate`` rejects fails its run before any Connect REST -call or ``application.properties`` write, rather than only when someone -remembered to validate first. +checks through :func:`iceberg_sink_preflight` for every build that has a plan +(one that declares ``sink.format: iceberg``, or an embedded Debezium build that +derives its sink), derived or hand-written, so such a contract ``fluid validate`` +rejects fails its run before any Connect REST call or ``application.properties`` +write. Which builds those are is ONE answer, :func:`iceberg_sink_plan`, read off the build the runner +executes (:func:`executing_build`): the validator, the preflight and both +runners consume it, so none of them re-implements another's gate. The catalog checks are TABLE-DRIVEN: each one reads the kind's row from ``providers/_iceberg_catalog.py`` (the classification the sink deriver, dbt @@ -34,18 +37,24 @@ * the kind must be in the table: an unknown value gets each emitter's historic fallback, and those fallbacks disagree with one another; +* on GCP the catalog must be named: absent, the sink writes whatever catalog + reaches the worker (REST by default, Glue by ``catalog-impl``...) while + dbt-bigquery and the GCP IaC create a BigLake table; * ``sink.catalog`` must agree with the expose's catalog, because dbt and the - IaC read only the expose; + IaC read only the expose, and so must the catalog an override or a + hand-written config selects (its ``type`` or ``catalog-impl``); * every ``binding.location`` key in the row's ``sink_requires`` must be set (Glue keeps its advisory region warning: the warehouse falls back); * the runtime must ship the catalog's client (the stock Apache Iceberg Kafka - Connect runtime has no Nessie client); + Connect runtime has no Nessie client, and the published sink predates the + ``bigquery`` catalog type); * an operator override must not move the warehouse away from the binding, nor leave the connector carrying both ``type`` and ``catalog-impl``. Debezium builds in ``bring-your-own`` / ``managed`` mode create only the SOURCE connector (no sink is derived), so none of this applies to them; an embedded -Debezium Server build is checked whenever it would derive an Iceberg sink. +Debezium Server build is checked whenever it writes an Iceberg sink, derived +or from its hand-written ``server.sink.config``. Why imperative Python and not JSON-Schema if/then: these are CROSS-OBJECT checks (a build's sink ↔ a different expose's binding; a computed warehouse vs an @@ -63,12 +72,14 @@ import logging from dataclasses import dataclass -from typing import AbstractSet, Any, Dict, List, Mapping, Optional, Sequence, Tuple +from typing import AbstractSet, Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple from ...providers._iceberg_catalog import ( + DYNAMODB_CATALOG_IMPL, FAMILY_GLUE, FAMILY_REST, FAMILY_UNKNOWN, + GLUE_CATALOG_IMPL, CatalogKind, binding_catalog_kind, canonical_catalog_kind, @@ -83,14 +94,46 @@ _Issues = Tuple[List[str], List[str]] -#: ``only_build`` default: check every build. A sentinel, not ``None``, because -#: a build with an explicit ``id: null`` runs with ``ctx.build_id is None``. -_ALL_BUILDS: Any = object() +#: The published Apache Iceberg Kafka Connect sink (Confluent Hub). Its +#: ``CatalogUtil`` knows no ``bigquery`` type; Iceberg 1.10 adds it. +PUBLISHED_KAFKA_CONNECT_SINK_VERSION = "1.9.2" + +#: The catalog each ``catalog-impl`` class is: Apache Iceberg's +#: ``CatalogUtil.ICEBERG_CATALOG_*`` classes (what each ``type`` loads) plus the +#: impl-only DynamoDB catalog. A ``catalog-impl`` that reaches the worker is +#: therefore compared as the catalog it is, not skipped as "no type". A class +#: not listed is a custom catalog: it matches no kind, BigLake included. +_IMPL_CATALOGS: Mapping[str, str] = { + GLUE_CATALOG_IMPL: "glue", + DYNAMODB_CATALOG_IMPL: "dynamodb", + "org.apache.iceberg.rest.RESTCatalog": "rest", + "org.apache.iceberg.nessie.NessieCatalog": "nessie", + "org.apache.iceberg.hive.HiveCatalog": "hive", + "org.apache.iceberg.jdbc.JdbcCatalog": "jdbc", + "org.apache.iceberg.hadoop.HadoopCatalog": "hadoop", + "org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog": "bigquery", +} + + +def executing_build(contract: Mapping[str, Any], build_id: Any) -> Optional[Mapping[str, Any]]: + """The build a runner whose run context carries ``build_id`` executes. + + ``build_acquisition_run_context`` ids a build ``build.get("id", "unknown")`` + (an explicit ``id: null`` runs as ``None``), so the same spelling selects it + here. Both streaming runners read their properties from this build and the + preflight checks this build, so the two can never look at different ones; + they used to read ``builds[0]`` while the preflight checked ``build_id``. + ``None`` when no build carries the id. + """ + for build in contract.get("builds") or []: + if isinstance(build, Mapping) and build.get("id", "unknown") == build_id: + return build + return None @dataclass(frozen=True) -class _SinkRuntime: - """How one build's runner assembles its Iceberg sink config. +class IcebergSinkPlan: + """How one build's runner assembles the Iceberg sink config it pushes. The two runtimes spell the catalog selector differently (Kafka Connect prefixes ``iceberg.catalog.``; Debezium Server takes bare keys) and merge @@ -100,6 +143,7 @@ class _SinkRuntime: engine: str #: Does the runner derive a catalog block from the expose for this build? + #: ``False`` means it pushes only the hand-written config, if there is one. derives: bool type_key: str impl_key: str @@ -109,10 +153,14 @@ class _SinkRuntime: warehouse_override: Tuple[str, Any] -def _sink_runtime(build: Mapping[str, Any]) -> Optional[_SinkRuntime]: - """The runtime that writes an Iceberg sink for ``build``, or ``None``. +def iceberg_sink_plan(build: Mapping[str, Any]) -> Optional[IcebergSinkPlan]: + """How ``build``'s runner writes its Iceberg sink, or ``None`` if it has none. - Mirrors each runner's own gate. The Debezium runner derives from + A plan exists for every build that declares an Iceberg sink or derives + one, whether the config is derived or hand-written. It is the ONE gate: + ``fluid validate`` checks a build exactly when this returns a plan, both + runners run :func:`iceberg_sink_preflight` exactly then, and they derive + exactly when ``plan.derives``. The Debezium runner derives from ``server.sink.type`` (default ``iceberg``) in embedded mode and never reads ``sink.format``, so selecting its builds by ``sink.format`` alone would let a deriving build skip every check while one that never derives (a @@ -136,7 +184,7 @@ def _sink_runtime(build: Mapping[str, Any]) -> Optional[_SinkRuntime]: return None config = server_sink.get("config") or {} label = "debezium.server.sink.config" - return _SinkRuntime( + return IcebergSinkPlan( engine=engine, derives=derives, type_key="type", @@ -159,7 +207,7 @@ def _sink_runtime(build: Mapping[str, Any]) -> Optional[_SinkRuntime]: overrides += (("iceberg_catalog_overrides", catalog_overrides),) if isinstance(handwritten, Mapping): overrides += (("sink_connector_config", handwritten),) - return _SinkRuntime( + return IcebergSinkPlan( engine=engine, derives=derives, type_key="iceberg.catalog.type", @@ -185,15 +233,19 @@ def iceberg_sink_preflight( ) -> Optional[str]: """Run-time twin of ``fluid validate``: ONE build's sink errors, or ``None``. - A runner calls this right before it derives a sink, so a contract the - validator rejects fails the run before any Connect REST call or file write - instead of shipping a connector that never starts (both selector keys) or - writes into a different catalog than dbt reads. ``build_id`` is the run - context's id (``build.get("id", "unknown")``); the build is selected by it, - not by matching message text. Warnings are logged and never fatal, the same - split as ``fluid validate`` without ``--strict``. + A runner calls this whenever :func:`iceberg_sink_plan` says it will push an + Iceberg sink config, derived or hand-written, so a contract the validator + rejects fails the run before any Connect REST call or file write instead of + shipping a connector that never starts (both selector keys) or writes into + a different catalog than dbt reads. ``build_id`` is the run context's id; + the build is :func:`executing_build`, the one the runner reads its + properties from, not one matched by message text. Warnings are logged and + never fatal, the same split as ``fluid validate`` without ``--strict``. """ - errors, warnings = _validate(contract, only_build=build_id) + build = executing_build(contract, build_id) + if build is None: + return None + errors, warnings = _validate(contract, builds=(build,)) for msg in warnings: (log or LOG).warning("iceberg_sink.preflight.warning build=%s %s", build_id, msg) if not errors: @@ -201,20 +253,18 @@ def iceberg_sink_preflight( return "iceberg sink preflight failed (see `fluid validate`): " + "; ".join(errors) -def _validate(contract: Mapping[str, Any], *, only_build: Any = _ALL_BUILDS) -> _Issues: +def _validate(contract: Mapping[str, Any], *, builds: Optional[Iterable[Any]] = None) -> _Issues: errors: List[str] = [] warnings: List[str] = [] iceberg_exposes = iceberg_sink_exposes(contract) expose_ids = {e.get("exposeId") or e.get("id") for e in iceberg_exposes} - for build in contract.get("builds") or []: + candidates = (contract.get("builds") or []) if builds is None else builds + for build in candidates: if not isinstance(build, Mapping): continue - # ``build_acquisition_run_context`` ids a build ``build.get("id", "unknown")``. - if only_build is not _ALL_BUILDS and build.get("id", "unknown") != only_build: - continue - runtime = _sink_runtime(build) + runtime = iceberg_sink_plan(build) if runtime is None: continue # not a build that writes an Iceberg sink _check_build(build, runtime, iceberg_exposes, expose_ids, errors, warnings) @@ -224,7 +274,7 @@ def _validate(contract: Mapping[str, Any], *, only_build: Any = _ALL_BUILDS) -> def _check_build( build: Mapping[str, Any], - runtime: _SinkRuntime, + runtime: IcebergSinkPlan, iceberg_exposes: Sequence[Mapping[str, Any]], expose_ids: AbstractSet[Any], errors: List[str], @@ -275,7 +325,7 @@ def _check_catalog( bid: Any, binding: Mapping[str, Any], sink: Mapping[str, Any], - runtime: _SinkRuntime, + runtime: IcebergSinkPlan, errors: List[str], warnings: List[str], ) -> None: @@ -299,22 +349,37 @@ def _check_catalog( return # The sink must write through the catalog dbt and the IaC read. Both read - # only the expose, so a ``sink.catalog`` that differs streams into one - # catalog while the static table and the models live in another. Compared - # canonically: ``iceberg-rest`` and ``rest`` are the same catalog. HARD. + # only the expose, so a ``sink.catalog``, an override or a hand-written + # config that writes another catalog streams into one catalog while the + # static table and the models live in another. Compared as the catalog the + # worker builds: ``iceberg-rest``, ``lakekeeper`` and ``type=rest`` are all + # the REST catalog. ONE error per cause, the most specific first. HARD. expose_kind = binding_catalog_kind(binding) - if sink_kind and sink_kind != expose_kind: - expose_source = ( - "binding.location.catalog" - if canonical_catalog_kind(loc.get("catalog")) - else f"the {binding.get('platform')!r} platform default" - ) + reaching, origin = _reaching_catalog(info, runtime) + if _gcp_catalog_unnamed(binding): + if reaching is not None and reaching != "bigquery": + _gcp_split(bid, sink, sink_kind, reaching, origin, runtime, errors) + elif sink_kind and sink_kind != expose_kind: errors.append( f"iceberg sink (build {bid!r}): sink.catalog {sink.get('catalog')!r} disagrees " - f"with the expose's catalog {expose_kind!r} ({expose_source}); the sink would " - f"write through {sink_kind} while dbt and the IaC read {expose_kind}. Set " + f"with the expose's catalog {expose_kind!r} ({_expose_source(binding)}); the sink " + f"would write through {sink_kind} while dbt and the IaC read {expose_kind}. Set " f"binding.location.catalog: {sink_kind} and drop sink.catalog" ) + elif ( + origin is not None + and reaching is not None + and reaching != _wire_catalog(catalog_kind_info(expose_kind)) + and not _carries_both_selectors(kind, info, runtime) + ): + label, key, value = origin + errors.append( + f"iceberg sink (build {bid!r}): {label} sets {key}={value!r}, so the sink would " + f"write through {_catalog_noun(reaching)} catalog while dbt and the IaC read the " + f"expose's catalog {expose_kind!r} ({_expose_source(binding)}). Declare the " + "catalog the sink writes to in binding.location.catalog; a REST endpoint that " + "fronts Glue (Glue's Iceberg REST endpoint) is catalog: rest" + ) # 4. catalog tagged-union completeness, from the row's ``sink_requires``. # HARD: forge can't derive a REST uri or a catalog name. Glue requires @@ -338,25 +403,157 @@ def _check_catalog( # Azure modules (Hive only in its -hive- distribution) but no iceberg-nessie # (apache/iceberg kafka-connect/build.gradle), so ``type=nessie`` fails to # load NessieCatalog on a stock worker. Advisory: a custom image may add it. - if info.name == "nessie" and runtime.engine == "kafka-connect": + if reaching == "nessie" and runtime.engine == "kafka-connect": warnings.append( f"iceberg sink (build {bid!r}): the stock Apache Iceberg Kafka Connect runtime " "does not bundle iceberg-nessie (apache/iceberg kafka-connect/build.gradle); " "add the iceberg-nessie jar to the worker's connector plugin directory, or " "the sink cannot load NessieCatalog" ) + # ``type=bigquery`` (BigQueryMetastoreCatalog) is in Iceberg's CatalogUtil + # only from 1.10; the sink published on Confluent Hub is older, so a stock + # worker fails to load the catalog at connector start. Advisory: a runtime + # built from Iceberg >= 1.10 has it. + if reaching == "bigquery" and runtime.engine == "kafka-connect": + warnings.append( + f"iceberg sink (build {bid!r}): the sink config sets " + "iceberg.catalog.type=bigquery, which the published Apache Iceberg Kafka Connect " + f"sink ({PUBLISHED_KAFKA_CONNECT_SINK_VERSION} on Confluent Hub) cannot load: " + "Iceberg's CatalogUtil gains the bigquery type in 1.10. Run a sink built from " + "Iceberg >= 1.10, or the connector fails at start" + ) _check_warehouse_override(bid, binding, sink, kind, info, runtime, warnings) _check_selector_overrides(bid, kind, info, runtime, errors) +def _reaching_catalog_type(info: Any, runtime: IcebergSinkPlan) -> Optional[str]: + """The catalog ``type`` the worker actually receives, or ``None``. + + The derived value when the runner derives (``None`` for a catalog reached by + ``catalog-impl``), then every override map in merge order, the last one + winning, as the runner merges them. A hand-written config therefore decides + for itself: its ``type`` is what reaches the worker, not the expose's kind. + """ + reaching: Optional[str] = None + if runtime.derives and not info.catalog_impl: + reaching = info.runtime_type + for _label, mapping in runtime.overrides: + if runtime.impl_key in mapping: + reaching = None + if runtime.type_key in mapping: + reaching = str(mapping[runtime.type_key] or "").strip().lower() or None + return reaching + + +def _wire_catalog(info: CatalogKind) -> str: + """The catalog the worker builds from ``info``'s derived selector. + + ``rest`` for every REST-family kind (lakekeeper, polaris, unity, + snowflake-managed), the kind itself for a native ``type``, and the kind a + ``catalog-impl`` class is (glue, dynamodb). + """ + if info.catalog_impl: + return _IMPL_CATALOGS.get(info.catalog_impl, info.catalog_impl) + return info.runtime_type or "" + + +def _reaching_catalog( + info: CatalogKind, runtime: IcebergSinkPlan +) -> Tuple[Optional[str], Optional[Tuple[str, str, str]]]: + """The catalog the worker builds, and the override that chose it. + + Starts from the derived kind (``info`` is :func:`iceberg_catalog_kind`'s + row, so ``sink.catalog`` counts) when the runner derives, then reads every + override map in merge order, the last one winning: a ``type`` names its + catalog, and a ``catalog-impl`` is the catalog its class is (a class + :data:`_IMPL_CATALOGS` does not know stays its own name, which matches no + kind). ``origin`` is ``(map label, key, value)`` of the override that + decided, ``None`` when the derived config did; the catalog is ``None`` + when nothing selects one. + """ + reaching: Optional[str] = _wire_catalog(info) if runtime.derives else None + origin: Optional[Tuple[str, str, str]] = None + for label, mapping in runtime.overrides: + if runtime.impl_key in mapping: + impl = str(mapping[runtime.impl_key] or "").strip() + reaching = _IMPL_CATALOGS.get(impl, impl) or None + origin = (label, runtime.impl_key, impl) + if runtime.type_key in mapping: + wire = str(mapping[runtime.type_key] or "").strip().lower() + reaching = wire or None + origin = (label, runtime.type_key, wire) + return reaching, origin + + +def _catalog_noun(catalog: str) -> str: + """``a REST`` / ``a glue`` / ``a com.acme.Custom``, for the messages.""" + return f"a {'REST' if catalog == 'rest' else catalog}" + + +def _expose_source(binding: Mapping[str, Any]) -> str: + """Where the expose's catalog comes from, for the disagreement messages.""" + loc = binding.get("location") or {} + if canonical_catalog_kind(loc.get("catalog")): + return "binding.location.catalog" + return f"the {binding.get('platform')!r} platform default" + + +def _gcp_split( + bid: Any, + sink: Mapping[str, Any], + sink_kind: str, + reaching: str, + origin: Optional[Tuple[str, str, str]], + runtime: IcebergSinkPlan, + errors: List[str], +) -> None: + """GCP is the one platform where an absent catalog means two things. + + dbt-bigquery and the GCP IaC read only an EXPLICIT ``location.catalog`` and + create a BigLake metastore table, while the sink writes through whatever + catalog type reaches the worker (the ``rest`` platform default, a + ``sink.catalog``, or a hand-written config). Unless that is BigLake too, + one table is written to two catalogs. Naming the expose's catalog is the only + way they agree. A catalog reached by ``catalog-impl`` (``sink.catalog: + glue``, a hand-written ``GlueCatalog``) counts like any other. HARD. + """ + if not runtime.derives: + source = "the hand-written sink config" + elif origin is not None: + source = origin[0] + elif sink_kind: + source = f"sink.catalog {sink.get('catalog')!r}" + else: + source = "the 'gcp' platform default" + errors.append( + f"iceberg sink (build {bid!r}): the GCP Iceberg expose sets no " + "binding.location.catalog, so it is read two ways: the sink would write through a " + f"{'REST' if reaching == 'rest' else reaching} catalog ({source}) while dbt-bigquery and " + "the GCP IaC, which read only " + "binding.location.catalog, create a BigLake metastore table. Set " + "binding.location.catalog: bigquery, or the REST kind your catalog is " + "(e.g. rest, lakekeeper)" + ) + + +def _gcp_catalog_unnamed(binding: Mapping[str, Any]) -> bool: + """Is ``binding`` a GCP Iceberg expose with no ``location.catalog``?""" + from ...iac.provider_match import canonical_cloud + + loc = binding.get("location") or {} + return canonical_cloud(binding.get("platform")) == "gcp" and not canonical_catalog_kind( + loc.get("catalog") + ) + + def _check_warehouse_override( bid: Any, binding: Mapping[str, Any], sink: Mapping[str, Any], kind: str, info: CatalogKind, - runtime: _SinkRuntime, + runtime: IcebergSinkPlan, warnings: List[str], ) -> None: """5. zero-drift cross-check (consumes PR1's same_warehouse). @@ -398,7 +595,7 @@ def _check_selector_overrides( bid: Any, kind: str, info: CatalogKind, - runtime: _SinkRuntime, + runtime: IcebergSinkPlan, errors: List[str], ) -> None: """6. the merged config must carry ``type`` XOR ``catalog-impl``. @@ -410,14 +607,7 @@ def _check_selector_overrides( override maps are merged last. Presence is what counts: CatalogUtil checks the key for null, so even an empty value trips it. HARD. """ - setters: Dict[str, str] = {} - if runtime.derives: - derived = runtime.impl_key if info.catalog_impl else runtime.type_key - setters[derived] = f"the derived {kind} catalog config" - for label, mapping in runtime.overrides: - for key in (runtime.type_key, runtime.impl_key): - if key in mapping: - setters[key] = label + setters = _selector_setters(kind, info, runtime) if runtime.type_key in setters and runtime.impl_key in setters: errors.append( f"iceberg sink (build {bid!r}): the sink config would carry both " @@ -426,3 +616,22 @@ def _check_selector_overrides( "with both type and catalog-impl set, so the sink never starts. Keep one, and " "change catalogs with binding.location.catalog rather than an override" ) + + +def _carries_both_selectors(kind: str, info: CatalogKind, runtime: IcebergSinkPlan) -> bool: + """Check 6's condition: the sink never starts, so no catalog reaches it.""" + setters = _selector_setters(kind, info, runtime) + return runtime.type_key in setters and runtime.impl_key in setters + + +def _selector_setters(kind: str, info: CatalogKind, runtime: IcebergSinkPlan) -> Dict[str, str]: + """``{selector key: who set it}`` in the merged config, the last setter winning.""" + setters: Dict[str, str] = {} + if runtime.derives: + derived = runtime.impl_key if info.catalog_impl else runtime.type_key + setters[derived] = f"the derived {kind} catalog config" + for label, mapping in runtime.overrides: + for key in (runtime.type_key, runtime.impl_key): + if key in mapping: + setters[key] = label + return setters diff --git a/fluid_build/build_runners/kafka_connect/runner.py b/fluid_build/build_runners/kafka_connect/runner.py index ea9eb1da..29067489 100644 --- a/fluid_build/build_runners/kafka_connect/runner.py +++ b/fluid_build/build_runners/kafka_connect/runner.py @@ -246,7 +246,22 @@ def _execute(ctx: RunContext, runner: KafkaConnectRunner) -> RunResult: started_at, t_start = begin_acquisition_run(ctx, runner) - props = ctx.contract.get("builds", [{}])[0].get("properties", {}) + from .iceberg_sink_validation import executing_build, iceberg_sink_plan + + # The build this run executes, the SAME one the Iceberg preflight checks. + # Reading ``builds[0]`` here ran a later build with the first build's + # connector settings while the preflight vetted the later build's. + build = executing_build(ctx.contract, ctx.build_id) + if build is None: + return _failed( + ctx, + started_at, + t_start, + f"kafka-connect: the run context's build {ctx.build_id!r} is not in the " + "contract's builds, so its properties cannot be read; run a build the " + "contract declares", + ) + props = build.get("properties") or {} kc_props = props.get("kafka-connect", {}) or {} deployment = kc_props.get("deployment", {}) or {} server_url = deployment.get("server_url") @@ -337,22 +352,24 @@ def _execute(ctx: RunContext, runner: KafkaConnectRunner) -> RunResult: # Optional sink-side connector (companion). When the build targets an # Iceberg sink, derive the iceberg.catalog.*/iceberg.tables.* config instead - # of forcing the operator to hand-author it (RFC §6.2). Gated: + # of forcing the operator to hand-author it (RFC §6.2). Gated by + # ``iceberg_sink_plan``, the one gate `fluid validate` reads too: # ``iceberg_sink_enabled`` defaults OFF when a hand-written - # ``sink_connector_config`` is already present, so existing contracts are - # byte-for-byte unaffected; when both are set the merge lets operator keys - # win (derived first). + # ``sink_connector_config`` is already present, so nothing is derived for + # it; when both are set the merge lets operator keys win (derived first). sink_config = kc_props.get("sink_connector_config") - iceberg_enabled = kc_props.get("iceberg_sink_enabled", sink_config is None) - if iceberg_enabled and str(ctx.sink.format or "").lower() == "iceberg": - # Fail closed BEFORE any Connect REST call: the same checks `fluid - # validate` runs, so a sink it rejects (a Lakekeeper binding with no - # uri, both catalog selectors set) never reaches the cluster. + sink_plan = iceberg_sink_plan(build) + if sink_plan is not None: + # Fail closed BEFORE any Connect REST call, for a derived AND a + # hand-written Iceberg sink: the same checks `fluid validate` runs, so + # a sink it rejects (a Lakekeeper binding with no uri, both catalog + # selectors set) never reaches the cluster. from .iceberg_sink_validation import iceberg_sink_preflight preflight_error = iceberg_sink_preflight(ctx.contract, ctx.build_id, log=LOG) if preflight_error: return _failed(ctx, started_at, t_start, preflight_error) + if sink_plan is not None and sink_plan.derives: binding = _find_iceberg_expose_binding(ctx.contract) if binding is not None: from ...providers._iceberg_catalog import resolve_iceberg_catalog diff --git a/fluid_build/cli/__init__.py b/fluid_build/cli/__init__.py index e78f49a7..ac586f1b 100644 --- a/fluid_build/cli/__init__.py +++ b/fluid_build/cli/__init__.py @@ -446,16 +446,31 @@ def main(argv: Optional[List[str]] = None) -> int: "exit_code": e.exit_code, }, ) - tag = f" [dim]\\[{slug}][/dim]" if slug else "" - context.console.print(f"[red]❌ {e.message}[/red]{tag}") + # The message, context values and suggestions are data, not markup: + # Rich reads a lowercase ``[word]`` as a style tag and drops it, so + # ``exposes[orders] names Iceberg catalog ...`` printed as ``exposes + # names ...`` and ``pip install 'pkg[gcp]'`` lost its extra. A ``Text`` + # styles the label and keeps the value literal; ``rich.markup.escape`` + # would not do, as it doubles a trailing backslash (``C:\work\``). + # soft_wrap on a context value: a remediation is a command, and Rich's + # word-wrap puts a real newline inside it, so pasting a printed + # ``tofu state rm
`` line runs it without its address. + from rich.text import Text + + message = Text(f"❌ {e.message}", style="red") + if slug: + message.append(f" [{slug}]", style="dim") + context.console.print(message) if e.context: for key, value in e.context.items(): - context.console.print(f" [dim]{key}:[/dim] {value}") + context.console.print( + Text.assemble(" ", (f"{key}:", "dim"), f" {value}"), soft_wrap=True + ) suggestions = getattr(e, "suggestions", None) or [] if suggestions: context.console.print("\n[yellow]💡 Suggestions:[/yellow]") for suggestion in suggestions: - context.console.print(f" • {suggestion}") + context.console.print(Text(f" • {suggestion}")) docs_url = getattr(e, "docs_url", None) if docs_url: # soft_wrap: a docs link is a URL, and Rich word-wraps at the terminal diff --git a/fluid_build/cli/_apply_opentofu_engine.py b/fluid_build/cli/_apply_opentofu_engine.py index edee1fe1..21d7d1fc 100644 --- a/fluid_build/cli/_apply_opentofu_engine.py +++ b/fluid_build/cli/_apply_opentofu_engine.py @@ -215,9 +215,10 @@ def apply_via_opentofu(args, logger: logging.Logger) -> int: contract, str(workdir), env, args, logger, plugin=plugin, actions=actions ) - # Pre-plan Iceberg catalog-move guard: Glue resources an older forge-cli - # created for an Iceberg table that lives in Lakekeeper / a REST catalog - # would otherwise be planned for DESTROY (iac/catalog_moves.py). + # Pre-plan Iceberg catalog-move guard: what an older forge-cli created for + # an Iceberg table that lives in Lakekeeper / a REST catalog (Glue resources + # on AWS, an EXTERNAL VOLUME on Snowflake) would otherwise be planned for + # DESTROY (iac/catalog_moves.py). _guard_catalog_moves(plugin, contract, provider, str(workdir), env, logger, actions=actions) _adopt_existing(plugin, contract, actions, str(workdir), env, logger) @@ -903,33 +904,67 @@ def _guard_catalog_moves( *, actions: Any = (), ) -> None: - """Fail closed when the plan would destroy Glue resources of an Iceberg table - that now lives in another catalog. + """Fail closed when the plan would destroy what an older forge-cli created for + an Iceberg table that now lives in another catalog (Glue resources on AWS, an + EXTERNAL VOLUME on Snowflake). Thin CLI adapter over ``iac.catalog_moves.guard_catalog_moves``, the shape of :func:`_guard_packaging_transitions`: detection and remediation text live in ``iac/``, and this side owns the ``CLIError`` and the audit event. - A no-op off AWS, for every contract without an AWS Iceberg expose in a - non-Glue catalog (checked before the state is even listed), and for a - fresh workdir. A probe that fails is logged and skipped rather than failing - the apply: the data-loss gate still stands behind it. + A no-op for a provider whose emitter moved nothing + (``catalog_moves.catalog_move_spec``), for every contract without a moved + expose (checked before the state is even listed), and for a fresh workdir. + A probe that cannot run is skipped rather than failing the apply, since + the data-loss gate still stands behind it, but never silently: that gate's + remedy is ``--allow-data-loss``, the one thing an operator must not reach + for here, so the skip is a WARNING that says so. """ - if provider != "aws": - return from fluid_build.iac.catalog_moves import ( CatalogMoveError, + catalog_move_spec, guard_catalog_moves, moved_iceberg_exposes, ) - if not moved_iceberg_exposes(contract): + spec = catalog_move_spec(plugin, provider) + if spec is None: + return + moved = moved_iceberg_exposes(contract, spec=spec) + if not moved: return + + def _skipped(reason: str) -> None: + warn( + logger, + "iceberg_catalog_move_probe_skipped", + provider=provider, + reason=reason, + exposes=[{"expose": e, "catalog": k} for e, k in moved], + remedy=( + "if the plan below destroys resources an older forge-cli created for these " + "exposes, do NOT pass --allow-data-loss: release them with `tofu state rm` " + "and re-run apply" + ), + ) + state = runner.tofu_state_list(workdir, env=env) if not state: + # ``tofu state list`` answers [] for a fresh workdir and for a failed + # listing alike; ``state pull`` tells the two apart. + try: + doc = read_state(Path(workdir), env) + except Exception as exc: # StateMigrationError, or no ``tofu`` at all + _skipped(f"the state could not be read: {exc}") + return + if doc.resources: + _skipped( + f"`tofu state list` listed nothing, but the state holds " + f"{len(doc.resources)} resource(s)" + ) return try: - guard_catalog_moves(plugin, contract, state, workdir=workdir, actions=actions) + guard_catalog_moves(plugin, contract, state, workdir=workdir, actions=actions, spec=spec) except CatalogMoveError as exc: # Structured audit event BEFORE the raise, as for a packaging transition. info(logger, "iceberg_catalog_move_blocked", **exc.event_fields()) @@ -939,7 +974,7 @@ def _guard_catalog_moves( {"kind": exc.kind, "error": str(exc), "remediation": list(exc.remediation)}, ) except Exception as exc: # defensive: a failed probe must not fail the apply - logger.debug("opentofu: catalog-move probe failed: %s", exc) + _skipped(f"the probe failed: {type(exc).__name__}: {exc}") def _report_suppressed_drift( diff --git a/fluid_build/cli/core.py b/fluid_build/cli/core.py index b06a4425..c358f465 100644 --- a/fluid_build/cli/core.py +++ b/fluid_build/cli/core.py @@ -157,15 +157,32 @@ def format_for_user(self, console: Console) -> None: (rustc/terraform-style) so operators and log scrapers see one routable code on every failure. """ + # The message, context and suggestions are data, not markup: Rich read + # ``exposes[orders]`` as a style tag and dropped it, and ``pip install + # 'pkg[gcp]'`` lost its extra. ``Text`` keeps them literal (``escape`` + # would double a trailing backslash, as in a Windows path). slug = getattr(self, "error_slug", None) - tag = f" [dim]\\[{slug}][/dim]" if slug else "" - console.print(f"[red]❌ {self.message}[/red]{tag}") + if not RICH_AVAILABLE: # the fallback Console has no markup to escape + console.print(f"❌ {self.message}" + (f" [{slug}]" if slug else "")) + if self.context: + console.print(f"Details: {self.context}") + for suggestion in self.suggestions or (): + console.print(f" • {suggestion}") + if self.docs_url: + console.print(f"Documentation: {self.docs_url}") + return + from rich.text import Text + + line = Text.assemble(("❌ " + str(self.message), "red")) + if slug: + line.append(f" [{slug}]", style="dim") + console.print(line, soft_wrap=True) if self.context: - console.print(f"[dim]Details: {self.context}[/dim]") + console.print(Text(f"Details: {self.context}", style="dim"), soft_wrap=True) if self.suggestions: console.print("\n[yellow]💡 Suggestions:[/yellow]") for suggestion in self.suggestions: - console.print(f" • {suggestion}") + console.print(Text(f" • {suggestion}"), soft_wrap=True) if self.docs_url: # soft_wrap: a docs link is a URL, and Rich word-wraps at the terminal # width, which puts a real newline INSIDE it. `_console.cprint_json` already diff --git a/fluid_build/cli/validate.py b/fluid_build/cli/validate.py index da7dcade..e1398d99 100644 --- a/fluid_build/cli/validate.py +++ b/fluid_build/cli/validate.py @@ -380,37 +380,50 @@ def run(args, logger: logging.Logger) -> int: duration = time.time() - start_time log_operation_failure(logger, "validate_contract", error=e.event, duration=duration) + # ``console_error`` wraps its text in ``[red]...[/red]`` markup, so every + # value echoed into it is escaped first: a path or message holding a + # lowercase ``[word]`` would otherwise lose it (see ``_cprint_literal``). + from rich.markup import escape + # Handle specific CLI errors with user-friendly messages if e.event == "version_below_minimum": if not args.quiet: console_error( - f"Contract version {e.context.get('version')} does not meet minimum requirement {e.context.get('constraint')}" + escape( + f"Contract version {e.context.get('version')} does not meet minimum requirement {e.context.get('constraint')}" + ) ) elif e.event == "version_above_maximum": if not args.quiet: console_error( - f"Contract version {e.context.get('version')} exceeds maximum allowed {e.context.get('constraint')}" + escape( + f"Contract version {e.context.get('version')} exceeds maximum allowed {e.context.get('constraint')}" + ) ) elif e.event == "contract_file_not_found": if not args.quiet: - console_error(f"Contract file not found: {e.context.get('path')}") + console_error(escape(f"Contract file not found: {e.context.get('path')}")) elif e.event == "contract_version_unsupported": if not args.quiet: console_error( - e.context.get( - "message", - f"Unsupported contract version: {e.context.get('version')}", + escape( + str( + e.context.get( + "message", + f"Unsupported contract version: {e.context.get('version')}", + ) + ) ) ) elif e.event == "contract_required": if not args.quiet: - console_error(f"{e.context.get('message', 'Contract file is required')}") + console_error(escape(f"{e.context.get('message', 'Contract file is required')}")) else: if not args.quiet: - console_error(f"Validation error: {e.event}") + console_error(escape(f"Validation error: {e.event}")) if e.context: for key, value in e.context.items(): - cprint(f" {key}: {value}") + _cprint_literal(f" {key}: {value}") # Stable slug + catalog-driven guidance (Error-UX card). ``e`` is # auto-enriched at construction, so catalogued events (contract_*, @@ -419,9 +432,9 @@ def run(args, logger: logging.Logger) -> int: if not args.quiet: slug = getattr(e, "error_slug", None) if slug: - cprint(f" [{slug}]") + _cprint_literal(f" [{slug}]") for suggestion in getattr(e, "suggestions", None) or []: - cprint(f" 💡 {suggestion}") + _cprint_literal(f" 💡 {suggestion}") docs_url = getattr(e, "docs_url", None) if docs_url: # soft_wrap: a docs link is a URL, and Rich word-wraps at the terminal @@ -1168,6 +1181,22 @@ def _cprint_json(payload: str) -> None: cprint(payload, soft_wrap=True, markup=False, highlight=False) +def _cprint_literal(line: str) -> None: + """Print a line that carries a validation message, with its ``[...]`` intact. + + ``cprint`` parses Rich markup, and Rich reads ``[orders]`` in + ``exposes[orders] declares ...`` as a style tag and drops it, so the CLI + named no expose at all (``exposes declares ...``) while ``--format json`` + named the right one. Any lowercase ``[word]`` goes the same way: a bundle + issue's ``[error]`` severity, an extra in ``pip install 'pkg[gcp]'``. + Messages are data, not markup, so ``markup=False``, as in + :func:`_cprint_json`. The redaction pass in ``cprint`` still runs. + ``soft_wrap`` leaves wrapping to the terminal, so Rich never breaks a line + inside an identifier or a copy-pasteable command. + """ + cprint(line, markup=False, soft_wrap=True) + + def _output_json_results(result: ValidationResult, args) -> int: """Output results in JSON format.""" import json @@ -1199,9 +1228,9 @@ def _output_text_results(result: ValidationResult, args, logger: logging.Logger) cprint("==================") for i, error in enumerate(result.errors, 1): if args.quiet: - cprint(f"ERROR: {error}") + _cprint_literal(f"ERROR: {error}") else: - cprint(f"{i:2}. {error}") + _cprint_literal(f"{i:2}. {error}") if not args.quiet: cprint() @@ -1211,7 +1240,7 @@ def _output_text_results(result: ValidationResult, args, logger: logging.Logger) cprint("Validation Warnings:") cprint("====================") for i, warning in enumerate(result.warnings, 1): - cprint(f"{i:2}. {warning}") + _cprint_literal(f"{i:2}. {warning}") cprint() # Verbose information @@ -1360,10 +1389,10 @@ def _try_workspace_validate( else: cprint(f" ❌ {product.name:<20} {len(result.errors)} error(s)") for err in result.errors[:3]: - cprint(f" {err}") + _cprint_literal(f" {err}") failed += 1 except Exception as exc: # noqa: BLE001 - cprint(f" ❌ {product.name:<20} load error: {exc}") + _cprint_literal(f" ❌ {product.name:<20} load error: {exc}") failed += 1 cprint("") @@ -1450,7 +1479,7 @@ def _run_bundle_validation( if issue.column is not None: loc += f":C{issue.column}" loc += ")" - cprint( + _cprint_literal( f" [{issue.severity}] {issue.validator}: {issue.file}{loc}: {issue.message}" ) diff --git a/fluid_build/iac/catalog_moves.py b/fluid_build/iac/catalog_moves.py index fbbe2279..4514e511 100644 --- a/fluid_build/iac/catalog_moves.py +++ b/fluid_build/iac/catalog_moves.py @@ -12,31 +12,57 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Pre-plan guard for an AWS Iceberg table whose catalog is not Glue. - -Until this release the AWS emitter ignored ``location.catalog`` and created a -Glue database and table for every Iceberg expose, ``catalog: lakekeeper`` -included, while the streaming sink wrote the table through Lakekeeper's REST -API. The emitter now creates no Glue resource for an Iceberg table another -catalog owns (``providers/aws.py::_glue_cataloged``). For a contract the old -release applied, those resources are in OpenTofu state and no longer in the -configuration, so the next ``tofu plan`` plans to DESTROY them. Destroying an -``aws_glue_catalog_database`` is Glue ``DeleteDatabase``, which deletes every -table in the database with it, tables created outside FLUID included. The -data-loss gate would stop that plan, but its remedy is ``--allow-data-loss``, -which is the one thing an operator must not do here. +"""Pre-plan guard for an Iceberg table whose catalog an emitter now honours. + +Until this release two emitters ignored, or misread, ``location.catalog``: + +* AWS created a Glue database and table for every Iceberg expose, + ``catalog: lakekeeper`` included, while the streaming sink wrote the table + through Lakekeeper's REST API. The emitter now creates no Glue resource for + an Iceberg table another catalog owns (``providers/aws.py::_glue_cataloged``). + Destroying an ``aws_glue_catalog_database`` is Glue ``DeleteDatabase``, + which deletes every table in the database with it, tables created outside + FLUID included. +* Snowflake tested the raw value against a hand-kept list that missed + ``lakekeeper`` (and ``bigquery``, and the ``iceberg-rest`` spelling), so + those exposes got a Snowflake EXTERNAL VOLUME and dbt wrote Snowflake-managed + tables onto it. The emitter now reads the shared catalog-kind table and + creates no volume for a catalog Snowflake does not manage. + +For a contract the old release applied, those resources are in OpenTofu state +and no longer in the configuration, so the next ``tofu plan`` plans to DESTROY +them. The data-loss gate would stop that plan, but its remedy is +``--allow-data-loss``, which is the one thing an operator must not do here. So, the same shape as the ownership-transition guard (``iac/transition.py``): the change is diffed against prior state *before* ``tofu plan``, and the apply fails closed with the ``tofu state rm`` commands that release this contract's -claim. ``state rm`` touches zero bytes in AWS; the Glue resources stay where -they are, for the operator to delete by hand once nothing reads them. - -Which addresses: the Glue resources the AWS plugin WOULD emit if every such -expose still named the Glue catalog, minus the ones it emits for the contract -as written (a Glue database a parquet expose still uses stays declared, so it -is not flagged), intersected with the state. The candidate list is the -plugin's own emit, so no address derivation is duplicated here. +claim. ``state rm`` touches zero bytes in the cloud; the resources stay where +they are, for the operator to delete by hand once nothing uses them. + +Which addresses: for each moved expose, the resources the plugin WOULD emit if +that one expose were still classified the old way, minus the ones it emits for +the contract as written (a Glue database a parquet expose still uses stays +declared, so it is not flagged), intersected with the state. Where a cloud has +a per-expose resource (the AWS Glue table), the expose's candidates count only +when that resource is in state too, which proves the old release created them +for THIS expose: a Glue database that a since-removed parquet expose created +is a plain removal, left to the data-loss gate. An expose whose old emit has +no per-expose resource (an Iceberg expose that names a database and no table) +cannot leave that proof, and the database was the only thing the old release +created for it, so its database counts on its own. The candidate list is the +plugin's own emit, so no address derivation is duplicated here, with one +exception: the Snowflake volume is keyed from the contract id alone, so its +address is derived from the id too (:func:`_snowflake_volume_before`). The +before image re-runs today's emitter over today's location, and an upgrade +that also changed that location (a warehouse that is now a catalog NAME, no +``iam_role_arn``) can no longer derive the volume the old release created. + +What changed is a fact about a previous release, not about the plugin as it is +now, so the in-tree clouds' :class:`CatalogMoveSpec` live in this module +(:data:`AWS_GLUE_MOVES`, :data:`SNOWFLAKE_VOLUME_MOVES`). An out-of-tree +plugin whose emitter makes the same kind of change declares its own with an +optional ``catalog_move_spec()`` method (see :func:`catalog_move_spec`). Pure except for the caller-supplied state listing and plugin; no ``tofu`` shell-out, no ``cli`` imports (the CLI layer owns ``CLIError`` translation and @@ -47,19 +73,87 @@ import copy import shlex -from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple +from dataclasses import dataclass +from typing import ( + Any, + Callable, + Dict, + Iterable, + List, + Mapping, + MutableMapping, + Optional, + Sequence, + Set, + Tuple, +) -from ..providers._iceberg_catalog import binding_catalog_kind, is_glue_cataloged +from ..providers._iceberg_catalog import ( + binding_catalog_kind, + catalog_kind_info, + iceberg_external_volume_is_override, + iceberg_external_volume_name, + is_glue_cataloged, + is_iceberg_format, +) +from .naming import safe_ident from .provider_match import is_cloud __all__ = [ + "AWS_GLUE_MOVES", "GLUE_CATALOG_RESOURCE_TYPES", + "SNOWFLAKE_VOLUME_MOVES", "CatalogMoveError", + "CatalogMoveSpec", + "catalog_move_spec", "detect_catalog_moves", "guard_catalog_moves", "moved_iceberg_exposes", ] + +@dataclass(frozen=True) +class CatalogMoveSpec: + """What one cloud's emitter stopped creating once ``location.catalog`` counted. + + ``moved`` picks the bindings the old and new emitters classify + differently; ``as_before`` rewrites a COPY of such a binding so the + current emitter classifies it the old way. ``resource_types`` are the + only resources the move removes from the configuration. + ``evidence_types`` are the per-expose ones among them: when non-empty, an + expose's candidates are flagged only if one of these is in state too, or + if the expose's old emit has none of them at all (nothing could prove it + then, and the rest was created for it alone). ``()`` means the cloud has + no per-expose resource, so every candidate in state counts. + ``addresses_before`` returns addresses the old release created for a moved + binding that do not depend on its location, for a resource the before + emit cannot re-derive once the location changed too; the default is none. + The prose fields fill the error message. + """ + + cloud: str + resource_types: Tuple[str, ...] + evidence_types: Tuple[str, ...] + moved: Callable[[Any], bool] + as_before: Callable[[MutableMapping[str, Any]], None] + #: Counted noun for the resources, e.g. "Glue catalog resource(s)". + noun: str + #: Why the plan would destroy them, and what that destroy does. + consequence: str + #: Where ``state rm`` leaves them, e.g. "AWS". + stays_in: str + #: What the operator does by hand afterwards, or instead. + afterwards: str + #: ``(contract, binding) -> addresses``; see the class docstring. + addresses_before: Callable[[Mapping[str, Any], Mapping[str, Any]], Iterable[str]] = ( + lambda contract, binding: () + ) + + +# --------------------------------------------------------------------------- +# AWS: Glue databases and tables +# --------------------------------------------------------------------------- + #: The resources the AWS emitter creates for a Glue-cataloged table, and the #: only ones a catalog move removes from the configuration. Lake Formation #: resources name the table too, but an Iceberg expose in another catalog has @@ -70,8 +164,160 @@ ) +def _moves_off_glue(binding: Any) -> bool: + """An AWS Iceberg binding with a Glue database whose table lives elsewhere. + + Only these change between the old emit and the new one: everything else + Glue catalogs (a parquet table, an Iceberg table with no ``catalog`` or + ``catalog: glue``) gets exactly the Glue resources it got before. + """ + if not isinstance(binding, Mapping) or not is_cloud(binding, "aws"): + return False + loc = binding.get("location") + if not isinstance(loc, Mapping) or not loc.get("database"): + return False + return not is_glue_cataloged(binding) + + +def _back_into_glue(binding: MutableMapping[str, Any]) -> None: + # The previous release created the Glue resources whatever + # ``location.catalog`` said, which is what naming Glue emits today. + binding["location"]["catalog"] = "glue" + + +AWS_GLUE_MOVES = CatalogMoveSpec( + cloud="aws", + resource_types=GLUE_CATALOG_RESOURCE_TYPES, + evidence_types=("aws_glue_catalog_table",), + moved=_moves_off_glue, + as_before=_back_into_glue, + noun="Glue catalog resource(s)", + consequence=( + "forge-cli no longer creates a Glue table for an Iceberg table in a non-Glue " + "catalog (it was a second claim on the table's name, without its metadata), so " + "applying now would plan to DESTROY these resources, and destroying a Glue " + "database deletes every table in it, including tables created outside FLUID." + ), + stays_in="AWS", + afterwards=( + "Delete a Glue table by hand once nothing reads it, and a Glue database only once " + "it holds nothing else. If the table belongs in Glue, remove location.catalog (or " + "set it to 'glue') instead." + ), +) + + +# --------------------------------------------------------------------------- +# Snowflake: EXTERNAL VOLUMEs +# --------------------------------------------------------------------------- + +#: The lower-cased ``location.catalog`` values the Snowflake emitter of +#: 0.19.0 and earlier created NO external volume for (``glue`` got a catalog +#: integration, the rest nothing). Every other value, ``lakekeeper`` and +#: ``bigquery`` included, got one. Frozen: it describes a released emitter. +_SNOWFLAKE_NO_VOLUME_BEFORE = frozenset( + {"glue", "polaris", "unity", "rest", "iceberg_rest", "nessie"} +) + +#: ``binding.format`` spellings the Snowflake emitter treats as Iceberg. +_SNOWFLAKE_ICEBERG_FORMATS = frozenset({"iceberg", "iceberg_table"}) + + +def _loses_snowflake_volume(binding: Any) -> bool: + """A Snowflake Iceberg binding that got an EXTERNAL VOLUME and gets none now. + + An operator-named volume (``icebergConfig.properties.external_volume``) + was never created by either emitter, so it cannot move. + """ + if not isinstance(binding, Mapping) or not is_cloud(binding, "snowflake"): + return False + fmt = binding.get("format") + if not (is_iceberg_format(fmt) or str(fmt or "").lower() in _SNOWFLAKE_ICEBERG_FORMATS): + return False + loc = binding.get("location") + if not isinstance(loc, Mapping) or iceberg_external_volume_is_override(binding): + return False + had_volume = str(loc.get("catalog") or "").lower() not in _SNOWFLAKE_NO_VOLUME_BEFORE + now_type = catalog_kind_info(binding_catalog_kind(binding)).snowflake_catalog_type + return had_volume and now_type != "built_in" + + +def _back_into_snowflake(binding: MutableMapping[str, Any]) -> None: + # The old emitter gave these the Snowflake-managed (built_in) branch; the + # volume name derives from the contract id, never from the catalog. + binding["location"]["catalog"] = "snowflake" + + +def _snowflake_volume_before( + contract: Mapping[str, Any], binding: Mapping[str, Any] +) -> Tuple[str, ...]: + """The EXTERNAL VOLUME address the old release created for ``contract``. + + The old emitter keyed it ``_vol_``, and the name + derives from the contract id too (``iac/providers/snowflake.py`` + ``_emit_iceberg_prereqs``; 0.19.0 used the same key), so the address + survives an upgrade that also changed the location: a ``warehouse`` that + is now a Lakekeeper warehouse NAME rather than an ``s3://`` URL, or no + ``iam_role_arn``. Re-running today's emitter derives no volume from that + location, yet the volume the old one created is still in state. + """ + cid = safe_ident(contract.get("id") or contract.get("name") or "product") + volume = iceberg_external_volume_name(contract, binding) + return (f"snowflake_external_volume.{safe_ident(f'{cid}_vol_{volume}')}",) + + +SNOWFLAKE_VOLUME_MOVES = CatalogMoveSpec( + cloud="snowflake", + resource_types=("snowflake_external_volume",), + # The volume is named per contract, not per expose, so no resource in + # state can say which expose it was created for. + evidence_types=(), + moved=_loses_snowflake_volume, + as_before=_back_into_snowflake, + noun="Snowflake EXTERNAL VOLUME(s)", + consequence=( + "forge-cli no longer creates an EXTERNAL VOLUME for an Iceberg table in a catalog " + "Snowflake does not manage (dbt now writes it as an externally cataloged table, " + "not a Snowflake-managed one on a volume), so applying now would plan to DROP " + "these volumes, and any Snowflake-managed Iceberg table an earlier dbt run wrote " + "onto one still uses it." + ), + stays_in="Snowflake", + afterwards=( + "Drop a volume by hand (DROP EXTERNAL VOLUME) only once no Iceberg table uses it. " + "If the table belongs in Snowflake's own catalog, remove location.catalog (or set " + "it to 'snowflake') instead." + ), + addresses_before=_snowflake_volume_before, +) + + +#: The in-tree clouds whose emitters changed, by plugin name. +_IN_TREE_SPECS: Mapping[str, CatalogMoveSpec] = { + AWS_GLUE_MOVES.cloud: AWS_GLUE_MOVES, + SNOWFLAKE_VOLUME_MOVES.cloud: SNOWFLAKE_VOLUME_MOVES, +} + + +def catalog_move_spec(plugin: Any, provider: Optional[str] = None) -> Optional[CatalogMoveSpec]: + """The :class:`CatalogMoveSpec` for ``plugin``, or ``None`` when its emitter + changed nothing. + + A plugin's own optional ``catalog_move_spec()`` method wins (an + out-of-tree cloud declares its move there); otherwise the in-tree table is + read by ``provider``, falling back to the plugin's ``name``. + """ + hook = getattr(plugin, "catalog_move_spec", None) + if callable(hook): + spec = hook() + # Only a real spec counts: anything else (a test double's auto-attribute, + # a plugin returning a dict) would be read field by field and guess. + return spec if isinstance(spec, CatalogMoveSpec) else None + return _IN_TREE_SPECS.get(str(provider or getattr(plugin, "name", "") or "")) + + class CatalogMoveError(RuntimeError): - """Prior state holds Glue resources for an Iceberg table now in another catalog. + """Prior state holds resources for an Iceberg table now in another catalog. ``kind`` is a stable, greppable tag (the ``PackagingTransitionError.kind`` discipline); ``remediation`` carries the ``tofu state rm`` commands. @@ -102,84 +348,117 @@ def event_fields(self) -> Dict[str, Any]: } -def _moves_off_glue(binding: Any) -> bool: - """An AWS Iceberg binding with a Glue database whose table lives elsewhere. +def _expose_label(exposure: Mapping[str, Any], index: int) -> Tuple[str, str]: + """``(expose id, catalog kind)``, as the message and the event name it.""" + expose_id = exposure.get("exposeId") or exposure.get("id") or index + return str(expose_id), binding_catalog_kind(exposure["binding"]) - Only these change between the old emit and the new one: everything else - Glue catalogs (a parquet table, an Iceberg table with no ``catalog`` or - ``catalog: glue``) gets exactly the Glue resources it got before. - """ - if not isinstance(binding, Mapping) or not is_cloud(binding, "aws"): - return False - loc = binding.get("location") - if not isinstance(loc, Mapping) or not loc.get("database"): - return False - return not is_glue_cataloged(binding) + +def _moved_indexes(contract: Mapping[str, Any], spec: CatalogMoveSpec) -> List[int]: + return [ + index + for index, exposure in enumerate(contract.get("exposes") or []) + if isinstance(exposure, Mapping) and spec.moved(exposure.get("binding")) + ] -def moved_iceberg_exposes(contract: Mapping[str, Any]) -> Tuple[Tuple[str, str], ...]: - """``(expose id, catalog kind)`` for every AWS Iceberg expose not in Glue. +def moved_iceberg_exposes( + contract: Mapping[str, Any], *, spec: CatalogMoveSpec = AWS_GLUE_MOVES +) -> Tuple[Tuple[str, str], ...]: + """``(expose id, catalog kind)`` for every expose ``spec`` says moved. Empty for every contract the guard cannot concern, which is the fast path: the apply engine reads no state and emits nothing for those. """ - out: List[Tuple[str, str]] = [] - for index, exposure in enumerate(contract.get("exposes") or []): - if not isinstance(exposure, Mapping): - continue - binding = exposure.get("binding") - if _moves_off_glue(binding): - expose_id = exposure.get("exposeId") or exposure.get("id") or index - out.append((str(expose_id), binding_catalog_kind(binding))) - return tuple(out) - + exposes = contract.get("exposes") or [] + return tuple(_expose_label(exposes[i], i) for i in _moved_indexes(contract, spec)) -def _as_glue(contract: Mapping[str, Any]) -> Dict[str, Any]: - """A copy of ``contract`` in which every moved expose names the Glue catalog. - What the previous release emitted for it: that release created the Glue - resources whatever ``location.catalog`` said. - """ +def _as_before(contract: Mapping[str, Any], index: int, spec: CatalogMoveSpec) -> Dict[str, Any]: + """A copy of ``contract`` in which only ``exposes[index]`` is classified the old way.""" doc = copy.deepcopy(dict(contract)) - for exposure in doc.get("exposes") or []: - if isinstance(exposure, Mapping) and _moves_off_glue(exposure.get("binding")): - exposure["binding"]["location"]["catalog"] = "glue" + spec.as_before(doc["exposes"][index]["binding"]) return doc -def _glue_addresses(resources: Mapping[str, Any]) -> set: +def _addresses(resources: Mapping[str, Any], types: Iterable[str]) -> Set[str]: return { f"{resource_type}.{name}" - for resource_type in GLUE_CATALOG_RESOURCE_TYPES + for resource_type in types for name in (resources.get(resource_type) or {}) } +def _detect( + plugin: Any, + contract: Mapping[str, Any], + state_addresses: Iterable[str], + spec: CatalogMoveSpec, + actions: Iterable[Mapping[str, Any]], +) -> Tuple[Tuple[str, ...], Tuple[Tuple[str, str], ...]]: + """The flagged addresses, sorted, and the moved exposes they were flagged for. + + One emit as written plus one per moved expose, so each candidate is + attributed to the expose whose old classification creates it. + """ + indexes = _moved_indexes(contract, spec) + if not indexes: + return (), () + in_state = {a.strip() for a in state_addresses or () if isinstance(a, str) and a.strip()} + if not in_state: + return (), () + actions = list(actions or ()) + now = _addresses(plugin.emit(contract, actions), spec.resource_types) + exposes = contract.get("exposes") or [] + flagged: Set[str] = set() + owners: List[Tuple[str, str]] = [] + for index in indexes: + before = _addresses( + plugin.emit(_as_before(contract, index, spec), actions), spec.resource_types + ) + before |= { + address + for address in spec.addresses_before(contract, exposes[index]["binding"]) + if address.split(".", 1)[0] in spec.resource_types + } + released = (before - now) & in_state + own = {a for a in before - now if a.split(".", 1)[0] in spec.evidence_types} + if own and not own & released: + # The expose has resources of its own and none of them is in + # state, so the previous release did not create the rest for it + # either. An expose with none (a database and no table) proves + # nothing either way: what it releases was created for it. + continue + if released: + flagged |= released + owners.append(_expose_label(exposes[index], index)) + return tuple(sorted(flagged)), tuple(owners) + + def detect_catalog_moves( plugin: Any, contract: Mapping[str, Any], state_addresses: Iterable[str], *, actions: Iterable[Mapping[str, Any]] = (), + spec: Optional[CatalogMoveSpec] = None, ) -> Tuple[str, ...]: - """The Glue addresses in state that the next plan would destroy, sorted. + """The addresses in state that the next plan would destroy, sorted. + + ``spec`` defaults to :func:`catalog_move_spec` for ``plugin``; a plugin + with none moves nothing. Matched exactly against ``tofu state list``: the module ``fluid`` writes has no child modules, so a ``module.``-prefixed address is not one its configuration declares and this emit change cannot be what removes it. - A provable no-op (no emit at all) for a contract with no AWS Iceberg expose - in another catalog, and for an empty state. + A provable no-op (no emit at all) for a contract with no moved expose, and + for an empty state. """ - if not moved_iceberg_exposes(contract): + spec = spec or catalog_move_spec(plugin) + if spec is None: return () - in_state = {a.strip() for a in state_addresses or () if isinstance(a, str) and a.strip()} - if not in_state: - return () - actions = list(actions or ()) - was = _glue_addresses(plugin.emit(_as_glue(contract), actions)) - now = _glue_addresses(plugin.emit(contract, actions)) - return tuple(sorted((was - now) & in_state)) + return _detect(plugin, contract, state_addresses, spec, actions)[0] def _state_rm_commands(addresses: Sequence[str], *, workdir: Optional[str]) -> Tuple[str, ...]: @@ -195,31 +474,29 @@ def guard_catalog_moves( *, workdir: Optional[str] = None, actions: Iterable[Mapping[str, Any]] = (), + spec: Optional[CatalogMoveSpec] = None, ) -> None: - """Raise :class:`CatalogMoveError` when the plan would destroy Glue resources of a + """Raise :class:`CatalogMoveError` when the plan would destroy resources of a moved table; a no-op otherwise (see :func:`detect_catalog_moves`).""" - addresses = detect_catalog_moves(plugin, contract, state_addresses, actions=actions) + spec = spec or catalog_move_spec(plugin) + if spec is None: + return + addresses, exposes = _detect(plugin, contract, state_addresses, spec, actions) if not addresses: return - exposes = moved_iceberg_exposes(contract) commands = _state_rm_commands(addresses, workdir=workdir) raise CatalogMoveError( "iceberg catalog move blocked — this contract's OpenTofu state holds " - f"{len(addresses)} Glue catalog resource(s) for Iceberg table(s) that now live in " - "another catalog:\n\n" + f"{len(addresses)} {spec.noun} for Iceberg table(s) that now live in another " + "catalog:\n\n" + "\n".join(f" exposes[{e}]: location.catalog {k}" for e, k in exposes) + "\n\n" + "\n".join(f" {a}" for a in addresses) - + "\n\nforge-cli no longer creates a Glue table for an Iceberg table in a non-Glue " - "catalog (it was a second claim on the table's name, without its metadata), so " - "applying now would plan to DESTROY these resources, and destroying a Glue " - "database deletes every table in it, including tables created outside FLUID.\n\n" + + f"\n\n{spec.consequence}\n\n" "Drop each from this contract's state, then re-run apply:\n\n" + "\n".join(f" {command}" for command in commands) + "\n\n`tofu state rm` touches ZERO bytes of infrastructure: the resources stay in " - "AWS, and only this contract's claim on them is released. Delete a Glue table by " - "hand once nothing reads it, and a Glue database only once it holds nothing else. If " - "the table belongs in Glue, remove location.catalog (or set it to 'glue') instead.", + f"{spec.stays_in}, and only this contract's claim on them is released. " + spec.afterwards, addresses=addresses, exposes=exposes, remediation=commands, diff --git a/fluid_build/iac/governance_validation.py b/fluid_build/iac/governance_validation.py index 75cafa9e..322c437f 100644 --- a/fluid_build/iac/governance_validation.py +++ b/fluid_build/iac/governance_validation.py @@ -80,12 +80,21 @@ def _aws_refusal( expose's error: escaping, it would end :func:`validate_governance`, and ``fluid validate`` would report no governance finding at all. """ + from ..providers._iceberg_catalog import FAMILY_UNKNOWN, catalog_kind_info, is_iceberg_format from .providers.aws import ( lf_governance, lf_tag_associations, refuse_lake_formation_on_external_catalog, ) + if ( + is_iceberg_format(binding.get("format")) + and catalog_kind_info(binding_catalog_kind(binding)).family == FAMILY_UNKNOWN + ): + # The unknown catalog value is the error (iceberg_validation reports + # it, and apply refuses it before any governance): a Lake Formation, + # column or row refusal naming the typo as a catalog is noise. + return "" try: # Lake Formation on an Iceberg table another catalog owns would # otherwise be dropped at apply: refused first, as the emitter does. diff --git a/fluid_build/iac/providers/aws.py b/fluid_build/iac/providers/aws.py index b054869d..62082502 100644 --- a/fluid_build/iac/providers/aws.py +++ b/fluid_build/iac/providers/aws.py @@ -800,6 +800,10 @@ def refuse_lake_formation_on_external_catalog( if is_glue_cataloged(binding): return kind = binding_catalog_kind(binding) + if catalog_kind_info(kind).family == FAMILY_UNKNOWN: + # A typo is not a catalog: refuse_unknown_iceberg_catalog names it, + # and "govern access in the catalog" would send the author astray. + return raise UnsupportedBindingError( "lake-formation-needs-glue-catalog", f"exposes[{_expose_id(exposure) or index}] declares governance.lakeFormation, but " diff --git a/fluid_build/providers/_iceberg_catalog.py b/fluid_build/providers/_iceberg_catalog.py index e432d00e..956bf1b9 100644 --- a/fluid_build/providers/_iceberg_catalog.py +++ b/fluid_build/providers/_iceberg_catalog.py @@ -168,6 +168,16 @@ def speaks_rest(self) -> bool: _UNKNOWN_KIND = CatalogKind("", FAMILY_UNKNOWN, "rest", None, "built_in") +class UnknownIcebergCatalogError(ValueError): + """An Iceberg ``location.catalog`` value no row of the table knows. + + Typed so a caller can tell the contract's refusal (the remedy is in the + message) from a planner that failed to run: the AWS provider used to log it + as ``plan_failed`` and then re-raise it, so the user read it twice, once as + a raw JSON log line. + """ + + def _fold(value: Any) -> str: return str(value or "").strip().lower().replace("_", "-") diff --git a/fluid_build/providers/aws/plan/planner.py b/fluid_build/providers/aws/plan/planner.py index 4d52d0ad..b82a200c 100644 --- a/fluid_build/providers/aws/plan/planner.py +++ b/fluid_build/providers/aws/plan/planner.py @@ -50,7 +50,7 @@ def _replacer(m: re.Match) -> str: return re.sub(r"\{\{\s*env\.(\S+?)\s*\}\}", _replacer, value) -def _glue_cataloged(binding: Mapping[str, Any]) -> bool: +def _glue_cataloged(binding: Mapping[str, Any], exposure: Mapping[str, Any]) -> bool: """Does this AWS binding's table live in the Glue Data Catalog? False only for an Iceberg binding whose ``location.catalog`` names a @@ -65,15 +65,55 @@ class where ``catalog: lakekeeper`` streamed over REST while the AWS plan (``providers/_iceberg_catalog.is_glue_cataloged``), imported lazily as ``_plan_exposures`` does its format helpers. A non-mapping ``location`` (a malformed contract this planner already tolerates) counts as no catalog. + + Raises ``ValueError`` for a catalog the table does not know + (:func:`_refuse_unknown_iceberg_catalog`): the answer decides whether a + Glue table exists, so it is refused rather than guessed. """ from fluid_build.providers._iceberg_catalog import is_glue_cataloged location = binding.get("location") if not isinstance(location, Mapping): location = {} + _refuse_unknown_iceberg_catalog(binding, location, exposure) return is_glue_cataloged({**binding, "location": location}) +def _refuse_unknown_iceberg_catalog( + binding: Mapping[str, Any], location: Mapping[str, Any], exposure: Mapping[str, Any] +) -> None: + """Refuse an Iceberg ``location.catalog`` value no row of the table knows. + + The planner's twin of the AWS IaC's ``refuse_unknown_iceberg_catalog``, + worded the same. ``fluid validate`` already reports the value, but nothing + guarantees it ran first, and an unknown kind classifies as "not Glue": a + typo (``catalog: glu``) used to plan the S3 buckets and no Glue database or + table, without a word. The remedy is in the message itself, because + ``AwsProvider.plan`` keeps this error's text and nothing else. + """ + from fluid_build.providers._iceberg_catalog import ( + FAMILY_UNKNOWN, + UnknownIcebergCatalogError, + catalog_kind_info, + is_iceberg_format, + known_catalog_kinds, + ) + + raw = location.get("catalog") + if not raw or not is_iceberg_format(binding.get("format")): + return + if catalog_kind_info(raw).family != FAMILY_UNKNOWN: + return + expose_id = exposure.get("exposeId") or exposure.get("id") or "?" + raise UnknownIcebergCatalogError( + f"exposes[{expose_id}] names Iceberg catalog {raw!r} (location.catalog), which " + "FLUID does not know, so it cannot tell whether the table belongs in AWS Glue, " + "and planning it would provision no Glue database or table. Use one of: " + + ", ".join(known_catalog_kinds()) + + ". Or remove location.catalog to keep the table in the Glue catalog." + ) + + def _declared_catalog(binding: Mapping[str, Any]) -> Any: """``location.catalog`` as written, for log events (``None`` when absent).""" location = binding.get("location") @@ -254,7 +294,7 @@ def _plan_infrastructure( platform = binding.get("platform", "").lower() # Handle different binding formats - if platform in ["aws", "glue", "athena"] and not _glue_cataloged(binding): + if platform in ["aws", "glue", "athena"] and not _glue_cataloged(binding, exposure): # A non-Glue Iceberg catalog owns its namespace, so no Glue # database (and no ``{account}-fluid-data`` fallback bucket, which # exists only to back that database). A bucket the binding names @@ -399,7 +439,7 @@ def _plan_iam_policies( continue platform = binding.get("platform", "").lower() - if platform in ["aws", "glue", "athena"] and not _glue_cataloged(binding): + if platform in ["aws", "glue", "athena"] and not _glue_cataloged(binding, exposure): # No Glue database exists for a non-Glue Iceberg table, so a Glue # IAM binding would grant on nothing while the catalog that owns # the table stays ungoverned. Say so rather than skip silently; @@ -563,7 +603,7 @@ def _plan_exposures( platform = binding.get("platform", "").lower() contract_schema = exposure.get("contract", {}).get("schema", []) - if platform in ["aws", "glue", "athena"] and not _glue_cataloged(binding): + if platform in ["aws", "glue", "athena"] and not _glue_cataloged(binding, exposure): # The table lives in its own Iceberg catalog (Lakekeeper, a REST # catalog...), whose writer creates it: no Glue Iceberg table here. logger.debug( diff --git a/fluid_build/providers/aws/provider.py b/fluid_build/providers/aws/provider.py index e7552622..389cb2ae 100644 --- a/fluid_build/providers/aws/provider.py +++ b/fluid_build/providers/aws/provider.py @@ -222,6 +222,15 @@ def plan( # Re-raise ProviderError as-is raise except Exception as e: + # Imported here: _iceberg_catalog imports this package's util, so a + # module-level import would be circular. + from fluid_build.providers._iceberg_catalog import UnknownIcebergCatalogError + + if isinstance(e, UnknownIcebergCatalogError): + # The contract's input was refused, and the message carries the + # remedy: not a planner failure. Logging it as ``plan_failed`` + # too printed it twice, once as a raw JSON line before the error. + raise ProviderError(str(e)) from e self.err_kv(event="plan_failed", contract_id=contract.get("id"), error=str(e)) # Wrap all other exceptions in ProviderError raise ProviderError(f"Failed to plan AWS deployment: {e}") from e diff --git a/tests/build_runners/test_debezium_full_matrix.py b/tests/build_runners/test_debezium_full_matrix.py index 5a360c56..0d124d05 100644 --- a/tests/build_runners/test_debezium_full_matrix.py +++ b/tests/build_runners/test_debezium_full_matrix.py @@ -68,7 +68,19 @@ def _base_contract( { "exposeId": "data", "kind": "table", - "binding": {"platform": "local", "format": "iceberg"}, + # A complete REST binding: the embedded runner preflights a + # hand-written Iceberg sink too, and `fluid validate` refuses a + # REST catalog with no uri or warehouse. + "binding": { + "platform": "local", + "format": "iceberg", + "location": { + "database": "bronze", + "table": "orders", + "uri": "http://rest:8181/catalog", + "warehouse": "analytics", + }, + }, "contract": {"schema": [], "schemaPolicy": "discover_and_freeze"}, } ], diff --git a/tests/build_runners/test_debezium_iceberg_sink.py b/tests/build_runners/test_debezium_iceberg_sink.py index e0fa76c3..6f422764 100644 --- a/tests/build_runners/test_debezium_iceberg_sink.py +++ b/tests/build_runners/test_debezium_iceberg_sink.py @@ -398,14 +398,96 @@ def test_embedded_complete_lakekeeper_derives_rest(tmp_path: Path): assert "debezium.sink.iceberg.catalog-impl=" not in text -def test_embedded_handwritten_config_skips_the_preflight(tmp_path: Path): - # Derivation is OFF for a hand-written config, so nothing is derived from - # the binding and the file stays what the operator wrote. +@pytest.mark.parametrize( + "sink, binding, expected", + [ + ( + {"type": "iceberg", "config": {"catalog.name": "rest"}}, + _LAKEKEEPER_NO_URI, + "lakekeeper catalog requires binding.location.uri", + ), + ( + {"type": "iceberg", "config": {"type": "rest", "catalog-impl": "x.Y"}}, + None, + "would carry both type", + ), + ], + ids=["missing-uri", "both-selectors"], +) +def test_embedded_handwritten_config_is_preflighted(tmp_path: Path, sink, binding, expected): + # Derivation is OFF for a hand-written config, but the server still writes + # an Iceberg sink, so the run refuses what `fluid validate` refuses. It + # used to write the file and boot a server that never started. + from fluid_build.api.runner import RunState + + contract = _contract(sink=sink, binding=binding) + result = _run_embedded(contract, tmp_path) + assert result.state == RunState.FAILED + assert expected in (result.error or ""), result.error + assert "iceberg sink preflight failed" in result.error + assert not _props_path(contract, tmp_path).exists() + + +def test_embedded_handwritten_config_validate_accepts_is_written_verbatim(tmp_path: Path): + # Preflighted, but nothing is derived: the file stays what the operator wrote. + complete = { + **_LAKEKEEPER_NO_URI, + "location": {**_LAKEKEEPER_NO_URI["location"], "uri": "http://lakekeeper:8181/catalog"}, + } contract = _contract( - sink={"type": "iceberg", "config": {"catalog.name": "rest"}}, binding=_LAKEKEEPER_NO_URI + sink={"type": "iceberg", "config": {"catalog.name": "rest"}}, binding=complete ) text = _props_text(contract, tmp_path) assert "debezium.sink.iceberg.catalog.name=rest" in text + assert "debezium.sink.iceberg.uri=" not in text + + +def test_multi_build_embedded_runner_uses_the_executing_builds_properties(tmp_path: Path): + # The runner used to read builds[0]'s Debezium settings for every build: + # here that is a hand-written config, so ingest derived nothing and wrote + # landing's catalog name under landing's server name. + import copy + + from fluid_build.build_runners.kafka_connect.iceberg_sink_validation import ( + iceberg_sink_preflight, + validate_iceberg_sink, + ) + + contract = _contract(sink={"type": "iceberg"}) + ingest = contract["builds"][0] + landing = copy.deepcopy(ingest) + landing["id"] = "landing" + landing["outputs"] = [] + landing["properties"]["sink"] = {"format": "parquet"} + landing["properties"]["debezium"]["server_name"] = "landing_srv" + landing["properties"]["debezium"]["server"]["sink"] = { + "type": "iceberg", + "config": {"catalog.name": "landing"}, + } + contract["builds"] = [landing, ingest] + assert validate_iceberg_sink(contract) == ([], []) + assert iceberg_sink_preflight(contract, "ingest") is None + + execute_debezium_build(ingest, contract, tmp_path, dry_run=False) + text = _props_path(contract, tmp_path).read_text() + assert "debezium.sink.iceberg.table-namespace=bronze" in text + assert "debezium.sink.iceberg.catalog-impl=org.apache.iceberg.aws.glue.GlueCatalog" in text + assert "landing" not in text + + +def test_embedded_runner_refuses_a_build_the_contract_does_not_declare(tmp_path: Path): + import copy + + from fluid_build.api.runner import RunState + from fluid_build.build_runners._acquisition_common import build_acquisition_run_context + from fluid_build.build_runners.debezium.runner import DebeziumRunner + + contract = _contract(sink={"type": "iceberg"}) + ghost = copy.deepcopy(contract["builds"][0]) + ghost["id"] = "ghost" + result = DebeziumRunner().run(build_acquisition_run_context(ghost, contract, tmp_path)) + assert result.state == RunState.FAILED + assert "build 'ghost' is not in the contract's builds" in result.error def test_bring_your_own_mode_is_not_preflighted(kafka_connect_mock, tmp_path: Path): diff --git a/tests/build_runners/test_iceberg_sink_deriver.py b/tests/build_runners/test_iceberg_sink_deriver.py index 2cfa1946..3a1c6ee6 100644 --- a/tests/build_runners/test_iceberg_sink_deriver.py +++ b/tests/build_runners/test_iceberg_sink_deriver.py @@ -303,11 +303,11 @@ def test_runner_default_off_when_handwritten_sink(kafka_connect_mock, tmp_path): } -def _run(contract, tmp_path): +def _run(contract, tmp_path, build=None): from fluid_build.build_runners._acquisition_common import build_acquisition_run_context from fluid_build.build_runners.kafka_connect.runner import KafkaConnectRunner - ctx = build_acquisition_run_context(contract["builds"][0], contract, tmp_path) + ctx = build_acquisition_run_context(build or contract["builds"][0], contract, tmp_path) return KafkaConnectRunner().run(ctx) @@ -393,15 +393,156 @@ def test_complete_lakekeeper_streams_over_rest(kafka_connect_mock, tmp_path): assert "iceberg.catalog.io-impl" not in cfg -def test_runner_handwritten_sink_skips_the_preflight(kafka_connect_mock, tmp_path): +# ── a hand-written Iceberg sink is preflighted too ────────────────────────── +# +# Derivation is OFF for a hand-written sink_connector_config, but the runner +# still PUSHES an Iceberg sink, so it runs the same checks `fluid validate` +# does. The gate used to be the derive flag: validate refused these contracts +# and the run POSTed them anyway, a connector that never started. + +_BOTH_SELECTORS = { + "connector.class": "org.apache.iceberg.connect.IcebergSinkConnector", + "topics": "public.orders", + "iceberg.catalog.type": "rest", + "iceberg.catalog.catalog-impl": "org.apache.iceberg.aws.glue.GlueCatalog", +} + + +@pytest.mark.parametrize( + "handwritten, mutate, expected", + [ + ( + {"connector.class": "x.Y", "topics": "t"}, + lambda c: c["exposes"][0].__setitem__("binding", _LAKEKEEPER_NO_URI), + "lakekeeper catalog requires binding.location.uri", + ), + (_BOTH_SELECTORS, lambda c: None, "would carry both iceberg.catalog.type"), + ( + {"connector.class": "x.Y", "topics": "t"}, + lambda c: c["builds"][0]["properties"]["kafka-connect"].__setitem__( + "streamingSink", {"upsertMode": True} + ), + "upsertMode is not supported in v1", + ), + ], + ids=["missing-uri", "both-selectors", "upsert"], +) +def test_runner_preflights_a_handwritten_iceberg_sink( + kafka_connect_mock, tmp_path, handwritten, mutate, expected +): + from fluid_build.api.runner import RunState + from fluid_build.build_runners.kafka_connect.iceberg_sink_validation import ( + validate_iceberg_sink, + ) + from fluid_build.build_runners.kafka_connect.runner import execute_kafka_connect_build + + contract = _iceberg_contract(sink_connector_config=handwritten) + mutate(contract) + # `fluid validate` refuses it ... + assert any(expected in e for e in validate_iceberg_sink(contract)[0]) + # ... and so does the run, before any Connect REST call. + result = _run(contract, tmp_path) + assert result.state == RunState.FAILED + assert expected in (result.error or ""), result.error + assert "iceberg sink preflight failed" in result.error + assert kafka_connect_mock.calls == [] + assert kafka_connect_mock.connectors == {} + + assert execute_kafka_connect_build(contract["builds"][0], contract, tmp_path) == 1 + assert kafka_connect_mock.calls == [] + + +def test_runner_handwritten_iceberg_sink_validate_accepts_is_pushed_verbatim( + kafka_connect_mock, tmp_path +): from fluid_build.build_runners.kafka_connect.runner import execute_kafka_connect_build - # Derivation is OFF for a hand-written sink, so nothing is derived from the - # binding and the run stays byte-for-byte what it was (validate still flags it). - contract = _iceberg_contract(sink_connector_config={"connector.class": "x.Y", "topics": "t"}) - contract["exposes"][0]["binding"] = _LAKEKEEPER_NO_URI + # Preflighted, but derivation stays OFF: nothing is merged in. + handwritten = { + "connector.class": "org.apache.iceberg.connect.IcebergSinkConnector", + "topics": "public.orders", + "iceberg.catalog.catalog-impl": "org.apache.iceberg.aws.glue.GlueCatalog", + } + contract = _iceberg_contract(sink_connector_config=handwritten) + assert execute_kafka_connect_build(contract["builds"][0], contract, tmp_path) == 0 + assert kafka_connect_mock.connectors["snk"]["config"] == handwritten + + +# ── multi-build: the runner reads the build it executes ───────────────────── +# +# Both runners used to read their properties from builds[0] while the +# preflight checked the build being run, so a later build ran with the first +# build's connector settings, overrides included, unchecked. + + +def _landing_then_ingest(): + """builds[0] lands Parquet with its own Connect settings; builds[1] streams + into the Iceberg expose.""" + import copy + + contract = _iceberg_contract() + ingest = contract["builds"][0] + landing = copy.deepcopy(ingest) + landing["id"] = "landing" + landing["outputs"] = [] + landing["properties"]["sink"] = {"format": "parquet"} + landing["properties"]["kafka-connect"].update( + connector_name="landing", + # Over the Glue expose this re-creates the both-selectors crash, if it + # ever reached ingest's derived config. + iceberg_catalog_overrides={"iceberg.catalog.type": "rest"}, + ) + contract["builds"] = [landing, ingest] + return contract + + +def test_multi_build_runner_uses_the_executing_builds_properties(kafka_connect_mock, tmp_path): + from fluid_build.build_runners.kafka_connect.iceberg_sink_validation import ( + iceberg_sink_preflight, + validate_iceberg_sink, + ) + from fluid_build.build_runners.kafka_connect.runner import execute_kafka_connect_build + + contract = _landing_then_ingest() + assert validate_iceberg_sink(contract) == ([], []) + assert iceberg_sink_preflight(contract, "ingest") is None + + assert execute_kafka_connect_build(contract["builds"][1], contract, tmp_path) == 0 + # ingest's own connector name, and ingest's own (absent) overrides. + assert set(kafka_connect_mock.connectors) == {"src", "src-sink"} + cfg = kafka_connect_mock.connectors["src-sink"]["config"] + assert cfg["iceberg.catalog.catalog-impl"] == "org.apache.iceberg.aws.glue.GlueCatalog" + assert "iceberg.catalog.type" not in cfg + + # And landing runs with ITS settings: a source connector, no sink. assert execute_kafka_connect_build(contract["builds"][0], contract, tmp_path) == 0 - assert kafka_connect_mock.connectors["snk"]["config"] == { - "connector.class": "x.Y", - "topics": "t", + assert set(kafka_connect_mock.connectors) == {"src", "src-sink", "landing"} + + +def test_multi_build_preflight_checks_the_build_the_runner_executes(kafka_connect_mock, tmp_path): + from fluid_build.api.runner import RunState + + # The reverse: ingest's OWN override is the bad one. builds[0] is clean. + contract = _landing_then_ingest() + contract["builds"][0]["properties"]["kafka-connect"].pop("iceberg_catalog_overrides") + contract["builds"][1]["properties"]["kafka-connect"]["iceberg_catalog_overrides"] = { + "iceberg.catalog.type": "rest" } + result = _run(contract, tmp_path, build=contract["builds"][1]) + assert result.state == RunState.FAILED + assert "build 'ingest'" in result.error and "would carry both" in result.error + assert kafka_connect_mock.calls == [] + + +def test_runner_refuses_a_build_the_contract_does_not_declare(kafka_connect_mock, tmp_path): + import copy + + from fluid_build.api.runner import RunState + + contract = _iceberg_contract() + ghost = copy.deepcopy(contract["builds"][0]) + ghost["id"] = "ghost" + result = _run(contract, tmp_path, build=ghost) + assert result.state == RunState.FAILED + assert "build 'ghost' is not in the contract's builds" in result.error + assert kafka_connect_mock.calls == [] diff --git a/tests/build_runners/test_iceberg_sink_validation.py b/tests/build_runners/test_iceberg_sink_validation.py index c1fd6ab1..48b8b504 100644 --- a/tests/build_runners/test_iceberg_sink_validation.py +++ b/tests/build_runners/test_iceberg_sink_validation.py @@ -19,11 +19,16 @@ import pytest from fluid_build.build_runners.kafka_connect.iceberg_sink_validation import ( + PUBLISHED_KAFKA_CONNECT_SINK_VERSION, + executing_build, + iceberg_sink_plan, iceberg_sink_preflight, validate_iceberg_sink, ) from fluid_build.providers._iceberg_catalog import ( + DYNAMODB_CATALOG_IMPL, FAMILY_REST, + GLUE_CATALOG_IMPL, canonical_catalog_kind, catalog_kind_info, known_catalog_kinds, @@ -444,7 +449,11 @@ def test_missing_required_warehouse_is_not_also_reported_as_divergence(): # presence is what CatalogUtil checks: an empty value still trips it (_GLUE_AWS, {"iceberg_catalog_overrides": {"iceberg.catalog.type": ""}}, True), # overriding the SAME key the deriver emits is fine - (_GLUE_AWS, {"iceberg_catalog_overrides": {"iceberg.catalog.catalog-impl": "x.Y"}}, False), + ( + _GLUE_AWS, + {"iceberg_catalog_overrides": {"iceberg.catalog.catalog-impl": GLUE_CATALOG_IMPL}}, + False, + ), (_REST, {"iceberg_catalog_overrides": {"iceberg.catalog.type": "rest"}}, False), # a hand-written sink config merged over a derived one (opt-in) ( @@ -588,3 +597,380 @@ def test_preflight_matches_the_run_context_id_of_an_id_less_build(): # build_acquisition_run_context ids it "unknown"; the message keeps "?". msg = iceberg_sink_preflight(contract, "unknown") assert msg is not None and "build '?'" in msg + + +# ── ONE gate: the plan validate, the preflight and both runners read ───────── + + +def test_executing_build_selects_by_the_run_context_id(): + contract = _sink_contract(_GLUE_AWS) + first = contract["builds"][0] + second = {**first, "id": "second"} + contract["builds"].append(second) + assert executing_build(contract, "ingest") is first + assert executing_build(contract, "second") is second + assert executing_build(contract, "absent") is None + del first["id"] + assert executing_build(contract, "unknown") is first # run-context spelling + first["id"] = None + assert executing_build(contract, None) is first + + +@pytest.mark.parametrize( + "kc, derives", + [ + ({}, True), + ({"sink_connector_config": {"topics": "t"}}, False), + ({"sink_connector_config": {"topics": "t"}, "iceberg_sink_enabled": True}, True), + ({"iceberg_sink_enabled": False}, False), + ], +) +def test_kafka_connect_plan_covers_derived_and_handwritten_sinks(kc, derives): + build = _sink_contract(_GLUE_AWS, kc=kc)["builds"][0] + plan = iceberg_sink_plan(build) + assert plan is not None and plan.derives is derives + + +def test_non_iceberg_kafka_connect_build_has_no_plan(): + build = _sink_contract(_GLUE_AWS, sink={"format": "parquet"})["builds"][0] + assert iceberg_sink_plan(build) is None + + +@pytest.mark.parametrize( + "dbz, derives", + [ + (_embedded(), True), + (_embedded({"config": {"catalog.name": "x"}}), False), + ({"deployment": {"mode": "bring-your-own"}}, None), + (_embedded({"type": "s3"}), None), + ], + ids=["derived", "handwritten", "source-only", "non-iceberg-server-sink"], +) +def test_debezium_plan_covers_derived_and_handwritten_sinks(dbz, derives): + build = _sink_contract(_GLUE_AWS, engine="debezium", debezium=dbz)["builds"][0] + plan = iceberg_sink_plan(build) + assert (None if plan is None else plan.derives) is derives + + +@pytest.mark.parametrize("kc", [{}, {"sink_connector_config": {"topics": "t"}}]) +def test_preflight_refuses_a_handwritten_sink_validate_refuses(kc): + contract = _sink_contract(_BROKEN, kc=kc) + assert _errs(contract) + msg = iceberg_sink_preflight(contract, "ingest") + assert msg is not None and "lakekeeper catalog requires binding.location.uri" in msg + + +# ── GCP: an unnamed catalog is read two ways ──────────────────────────────── + +_GCP_REST_LOC = {"uri": "https://rest.example/catalog", "warehouse": "gs://acme-lake/wh"} + + +@pytest.mark.parametrize("engine", ["kafka-connect", "debezium"]) +def test_gcp_expose_with_no_catalog_errors_naming_both_readings(engine): + contract = _sink_contract( + _loc_binding(platform="gcp", **_GCP_REST_LOC), + engine=engine, + debezium=_embedded() if engine == "debezium" else None, + ) + hit = [e for e in _errs(contract) if "sets no binding.location.catalog" in e] + assert len(hit) == 1, _errs(contract) + msg = hit[0] + assert "a REST catalog (the 'gcp' platform default)" in msg + assert "dbt-bigquery and the GCP IaC" in msg and "BigLake metastore table" in msg + assert "Set binding.location.catalog: bigquery" in msg and "lakekeeper" in msg + + +def test_gcp_rest_sink_catalog_does_not_name_the_expose_catalog(): + # dbt-bigquery and the IaC never read sink.catalog, so it cannot settle it. + contract = _sink_contract( + _loc_binding(platform="gcp", **_GCP_REST_LOC), sink={"catalog": "rest"} + ) + hit = [e for e in _errs(contract) if "sets no binding.location.catalog" in e] + assert len(hit) == 1 and "a REST catalog (sink.catalog 'rest')" in hit[0] + + +def test_gcp_sink_catalog_bigquery_agrees_with_biglake_and_warns_the_runtime(): + # sink.catalog bigquery reaches BigLake, and so do dbt-bigquery and the GCP + # IaC for an absent location.catalog: one catalog, so no split and no + # "disagrees with 'rest'" error (dbt and the IaC never read 'rest' here). + # The published sink still cannot load type=bigquery, which is a warning. + contract = _sink_contract( + _loc_binding(platform="gcp", warehouse="gs://acme/wh"), sink={"catalog": "bigquery"} + ) + errors, warnings = validate_iceberg_sink(contract) + assert errors == [] + assert any("iceberg.catalog.type=bigquery" in w for w in warnings), warnings + + +def test_gcp_handwritten_rest_sink_is_a_split_named_after_the_handwritten_config(): + contract = _sink_contract( + _loc_binding(platform="gcp", **_GCP_REST_LOC), + kc={"sink_connector_config": {"iceberg.catalog.type": "rest"}}, + ) + hit = [e for e in _errs(contract) if "sets no binding.location.catalog" in e] + assert len(hit) == 1 and "a REST catalog (the hand-written sink config)" in hit[0], hit + + +def test_gcp_handwritten_bigquery_sink_is_not_a_split(): + contract = _sink_contract( + _loc_binding(platform="gcp", warehouse="gs://acme/wh"), + kc={"sink_connector_config": {"iceberg.catalog.type": "bigquery"}}, + ) + assert not any("sets no binding.location.catalog" in e for e in _errs(contract)) + + +def test_bigquery_warning_follows_the_type_that_reaches_the_worker(): + # A hand-written config that sets type=rest over a bigquery expose sends + # REST to the worker, so the "cannot load bigquery" warning would be false. + contract = _sink_contract( + _loc_binding("bigquery", platform="gcp", warehouse="gs://acme/wh"), + kc={"sink_connector_config": {"iceberg.catalog.type": "rest"}}, + ) + errors, warnings = validate_iceberg_sink(contract) + assert not any("iceberg.catalog.type=bigquery" in w for w in warnings), warnings + # ... and REST is not the bigquery catalog dbt and the IaC read (JRN-707-2). + assert any("Declare the catalog the sink writes to" in e for e in errors), errors + + +@pytest.mark.parametrize( + "catalog, location", + [ + ("bigquery", {"warehouse": "gs://acme/wh"}), + ("rest", _GCP_REST_LOC), + ("lakekeeper", COMPLETE), + ], +) +def test_gcp_expose_naming_its_catalog_is_not_refused(catalog, location): + errs = _errs(_sink_contract(_loc_binding(catalog, platform="gcp", **location))) + assert errs == [] + + +@pytest.mark.parametrize( + "sink, kc, reaching, source", + [ + # glue is in the acquisitionSink.catalog schema enum, reached by catalog-impl + ({"catalog": "glue"}, None, "a glue", "sink.catalog 'glue'"), + ( + None, + {"sink_connector_config": {"iceberg.catalog.catalog-impl": GLUE_CATALOG_IMPL}}, + "a glue", + "the hand-written sink config", + ), + ( + None, + {"sink_connector_config": {"iceberg.catalog.catalog-impl": DYNAMODB_CATALOG_IMPL}}, + "a dynamodb", + "the hand-written sink config", + ), + ( + None, + {"sink_connector_config": {"iceberg.catalog.catalog-impl": "com.acme.LakeCatalog"}}, + "a com.acme.LakeCatalog", + "the hand-written sink config", + ), + ( + {"catalog": "bigquery"}, + {"iceberg_catalog_overrides": {"iceberg.catalog.type": "rest"}}, + "a REST", + "iceberg_catalog_overrides", + ), + ], + ids=[ + "sink-catalog-glue", + "handwritten-glue-impl", + "handwritten-dynamodb-impl", + "handwritten-custom-impl", + "override-type-over-sink-catalog", + ], +) +def test_gcp_split_covers_a_catalog_reached_by_catalog_impl(sink, kc, reaching, source): + """CON-2 / LOGIC-2: the gate used to key on the wire ``type``, which is + absent for a catalog reached by ``catalog-impl``, so the sink committed to + Glue (or DynamoDB, or a custom class) while dbt-bigquery and the GCP IaC + created a BigLake table. Every catalog but BigLake is refused.""" + contract = _sink_contract( + _loc_binding(platform="gcp", warehouse="gs://acme/wh"), sink=sink, kc=kc + ) + errs = _errs(contract) + hit = [e for e in errs if "sets no binding.location.catalog" in e] + assert len(hit) == 1, errs + assert f"{reaching} catalog ({source})" in hit[0] + assert not any("disagrees" in e or "Declare the catalog" in e for e in errs), errs + assert iceberg_sink_preflight(contract, "ingest") is not None + + +@pytest.mark.parametrize( + "kc", + [ + {"sink_connector_config": {"iceberg.catalog.type": "bigquery"}}, + { + "sink_connector_config": { + "iceberg.catalog.catalog-impl": ( + "org.apache.iceberg.gcp.bigquery.BigQueryMetastoreCatalog" + ) + } + }, + ], + ids=["type", "catalog-impl"], +) +def test_gcp_handwritten_biglake_is_not_a_split_by_type_or_impl(kc): + contract = _sink_contract(_loc_binding(platform="gcp", warehouse="gs://acme/wh"), kc=kc) + assert not any("sets no binding.location.catalog" in e for e in _errs(contract)) + + +# ── a hand-written selector must agree with a NAMED expose catalog ─────────── + + +def _disagreements(contract): + return [e for e in _errs(contract) if "Declare the catalog the sink writes to" in e] + + +@pytest.mark.parametrize( + "binding, kc, engine, label, setting, expose", + [ + # following the GCP split error's remedy, while the config still writes REST + ( + _loc_binding("bigquery", platform="gcp", warehouse="gs://acme/wh"), + {"sink_connector_config": {"iceberg.catalog.type": "rest"}}, + "kafka-connect", + "sink_connector_config", + "iceberg.catalog.type='rest'", + "'bigquery' (binding.location.catalog)", + ), + # the AWS Glue default, written over REST (a REST endpoint fronting Glue) + ( + _GLUE_AWS, + {"sink_connector_config": {"iceberg.catalog.type": "rest"}}, + "kafka-connect", + "sink_connector_config", + "iceberg.catalog.type='rest'", + "'glue' (the 'aws' platform default)", + ), + # an override over a derived REST config + ( + _loc_binding("lakekeeper", **COMPLETE), + {"iceberg_catalog_overrides": {"iceberg.catalog.type": "nessie"}}, + "kafka-connect", + "iceberg_catalog_overrides", + "iceberg.catalog.type='nessie'", + "'lakekeeper' (binding.location.catalog)", + ), + # the Snowflake-managed default, written through Glue + ( + _loc_binding(platform="snowflake", **COMPLETE), + {"sink_connector_config": {"iceberg.catalog.catalog-impl": GLUE_CATALOG_IMPL}}, + "kafka-connect", + "sink_connector_config", + f"iceberg.catalog.catalog-impl='{GLUE_CATALOG_IMPL}'", + "'snowflake-managed' (the 'snowflake' platform default)", + ), + ( + _loc_binding("bigquery", platform="gcp", warehouse="gs://acme/wh"), + None, + "debezium", + "debezium.server.sink.config", + "type='rest'", + "'bigquery' (binding.location.catalog)", + ), + ], + ids=["gcp-remedy-followed", "aws-glue-default", "override", "snowflake-default", "debezium"], +) +def test_a_handwritten_catalog_that_differs_from_the_named_one_is_refused( + binding, kc, engine, label, setting, expose +): + """JRN-707-2: the sink would write one catalog while dbt and the IaC read + another, and validate and the preflight both passed.""" + debezium = _embedded({"config": {"type": "rest", "uri": "http://lk:8181/catalog"}}) + contract = _sink_contract( + binding, kc=kc, engine=engine, debezium=debezium if engine == "debezium" else None + ) + hit = _disagreements(contract) + assert len(hit) == 1, _errs(contract) + assert hit[0].startswith(f"iceberg sink (build 'ingest'): {label} sets {setting}") + assert f"the expose's catalog {expose}" in hit[0] + assert "is catalog: rest" in hit[0] + msg = iceberg_sink_preflight(contract, "ingest") + assert msg is not None and "Declare the catalog the sink writes to" in msg + + +@pytest.mark.parametrize( + "binding, kc", + [ + # every REST-family kind is type=rest on the wire + ( + _loc_binding("lakekeeper", **COMPLETE), + {"sink_connector_config": {"iceberg.catalog.type": "rest"}}, + ), + ( + _loc_binding("rest", **COMPLETE), + {"sink_connector_config": {"iceberg.catalog.type": "REST"}}, + ), + ( + _loc_binding(platform="snowflake", **COMPLETE), + {"sink_connector_config": {"iceberg.catalog.type": "rest"}}, + ), + # Glue by type or by its class + (_GLUE_AWS, {"sink_connector_config": {"iceberg.catalog.type": "glue"}}), + (_GLUE_AWS, {"sink_connector_config": {"iceberg.catalog.catalog-impl": GLUE_CATALOG_IMPL}}), + # a REST endpoint that fronts Glue, declared as what the sink writes to + ( + _loc_binding("rest", platform="aws", **COMPLETE), + {"sink_connector_config": {"iceberg.catalog.type": "rest"}}, + ), + # a hand-written config that selects no catalog cannot disagree + (_GLUE_AWS, {"sink_connector_config": {"topics": "t"}}), + ], +) +def test_a_handwritten_catalog_that_matches_the_named_one_passes(binding, kc): + assert _disagreements(_sink_contract(binding, kc=kc)) == [] + + +def test_sink_catalog_disagreement_is_reported_once(): + # sink.catalog and the hand-written type both disagree: one cause, one error + contract = _sink_contract( + _loc_binding("bigquery", platform="gcp", warehouse="gs://acme/wh"), + sink={"catalog": "rest"}, + kc={ + "iceberg_sink_enabled": True, + "sink_connector_config": {"iceberg.catalog.type": "rest"}, + }, + ) + errs = _errs(contract) + assert len([e for e in errs if "disagrees" in e]) == 1, errs + assert _disagreements(contract) == [], errs + + +def test_both_selectors_draw_only_the_selector_error(): + # The sink never starts, so no catalog reaches it to disagree with. + contract = _sink_contract( + _GLUE_AWS, kc={"iceberg_catalog_overrides": {"iceberg.catalog.type": "rest"}} + ) + errs = _errs(contract) + assert any("would carry both" in e for e in errs), errs + assert _disagreements(contract) == [], errs + + +def test_unnamed_catalog_off_gcp_is_not_refused(): + assert _errs(_sink_contract(_loc_binding(**COMPLETE))) == [] + + +# ── bigquery: the published Kafka Connect sink predates the type ──────────── + + +def test_bigquery_on_kafka_connect_warns_about_the_published_sink(): + contract = _sink_contract(_loc_binding("bigquery", platform="gcp", warehouse="gs://acme/wh")) + errors, warnings = validate_iceberg_sink(contract) + assert errors == [] + hit = [w for w in warnings if "iceberg.catalog.type=bigquery" in w] + assert len(hit) == 1 + assert PUBLISHED_KAFKA_CONNECT_SINK_VERSION in hit[0] and "1.10" in hit[0] + assert "Confluent Hub" in hit[0] + + +def test_bigquery_on_debezium_server_has_no_kafka_connect_warning(): + contract = _sink_contract( + _loc_binding("bigquery", platform="gcp", warehouse="gs://acme/wh"), + engine="debezium", + debezium=_embedded(), + ) + assert not any("iceberg.catalog.type=bigquery" in w for w in _warns(contract)) diff --git a/tests/cli/test_cli_output_keeps_brackets.py b/tests/cli/test_cli_output_keeps_brackets.py new file mode 100644 index 00000000..6e787ea7 --- /dev/null +++ b/tests/cli/test_cli_output_keeps_brackets.py @@ -0,0 +1,381 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""The CLI prints validation and error messages literally, ``[...]`` included. + +The console renders through Rich, which reads a lowercase ``[word]`` as a style +tag and drops it. Every Iceberg catalog message names its expose as +``exposes[]``, so ``fluid validate`` printed ``exposes declares +governance.lakeFormation`` while ``--format json`` printed ``exposes[orders] +declares ...``, and a ``CLIError``'s context (``error: exposes names Iceberg +catalog ...``) lost the id the same way. These tests drive the real entry point +(``fluid_build.cli.main``) and assert on what reaches stdout. +""" + +from __future__ import annotations + +import json +import logging +import os +from pathlib import Path +from types import SimpleNamespace +from typing import Any, Dict + +import pytest +import yaml + +import fluid_build.cli as cli_mod +import fluid_build.cli.validate as validate_mod +from fluid_build.cli._common import CLIError + +pytestmark = pytest.mark.unit + +# What Rich would read as markup, and so drop, if printed through it. +MARKUP_LIKE = "the [bold]literal[/bold] [orders] [gcp] text" + + +@pytest.fixture(autouse=True) +def _clean_env(monkeypatch, tmp_path): + """No stale FLUID_ env vars, every command enabled, cwd in tmp_path.""" + for key in list(os.environ): + if key.startswith("FLUID_"): + monkeypatch.delenv(key, raising=False) + monkeypatch.setenv("FLUID_BUILD_PROFILE", "experimental") + monkeypatch.chdir(tmp_path) + + +def _contract(catalog: str, *, lake_formation: bool) -> Dict[str, Any]: + binding: Dict[str, Any] = { + "platform": "aws", + "format": "iceberg", + "location": { + "catalog": catalog, + "uri": "http://lakekeeper:8181/catalog", + "warehouse": "analytics", + "database": "streaming", + "table": "orders", + "bucket": "acme-lake", + "region": "eu-west-1", + }, + } + if lake_formation: + binding["governance"] = { + "lakeFormation": { + "grants": [ + { + "principal": "arn:aws:iam::123456789012:role/analyst", + "permissions": ["SELECT"], + } + ] + } + } + return { + "fluidVersion": "0.7.5", + "kind": "DataProduct", + "id": "bronze.orders_stream", + "name": "Orders stream", + "domain": "sales", + "metadata": {"layer": "Bronze", "owner": {"team": "data-platform"}}, + "exposes": [ + { + "exposeId": "orders", + "kind": "table", + "binding": binding, + "contract": { + "schema": [ + {"name": "order_id", "type": "integer", "required": True}, + {"name": "amount_cents", "type": "integer"}, + ] + }, + } + ], + } + + +def _write(tmp_path: Path, contract: Dict[str, Any]) -> str: + path = tmp_path / "contract.fluid.yaml" + path.write_text(yaml.safe_dump(contract, sort_keys=False), encoding="utf-8") + return str(path) + + +def _unwrapped(text: str) -> str: + """The output with Rich's word-wrap newlines folded back into spaces.""" + return " ".join(text.split()) + + +# --------------------------------------------------------------------------- +# fluid validate: the error list +# --------------------------------------------------------------------------- + + +def test_validate_names_the_expose_in_its_error(tmp_path, capsys): + path = _write(tmp_path, _contract("lakekeeper", lake_formation=True)) + + rc = cli_mod.main(["validate", path]) + + out = capsys.readouterr().out + assert rc == 1 + assert "exposes[orders] declares governance.lakeFormation" in _unwrapped(out) + + +def test_validate_quiet_names_the_expose_in_its_error(tmp_path, capsys): + path = _write(tmp_path, _contract("lakekeeper", lake_formation=True)) + + rc = cli_mod.main(["validate", "--quiet", path]) + + out = capsys.readouterr().out + assert rc == 1 + assert "ERROR: exposes[orders] declares governance.lakeFormation" in _unwrapped(out) + + +def test_validate_json_is_unchanged(tmp_path, capsys): + """The JSON path already printed literally; it must still parse and name the id.""" + path = _write(tmp_path, _contract("lakekeeper", lake_formation=True)) + + rc = cli_mod.main(["validate", "--format", "json", path]) + + doc = json.loads(capsys.readouterr().out) + assert rc == 1 + assert doc["valid"] is False + assert doc["errors"][0].startswith("exposes[orders] declares governance.lakeFormation") + + +def _result(errors, warnings) -> SimpleNamespace: + return SimpleNamespace( + errors=list(errors), + warnings=list(warnings), + is_valid=not errors, + schema_version="0.7.5", + validation_time=0.0, + get_summary=lambda: "summary", + ) + + +@pytest.mark.parametrize("quiet", [False, True]) +def test_validate_prints_markup_like_messages_verbatim(capsys, quiet): + args = SimpleNamespace(quiet=quiet, verbose=False, strict=False) + + validate_mod._output_text_results( + _result([f"error: {MARKUP_LIKE}"], [f"warning: {MARKUP_LIKE}"]), + args, + logging.getLogger("test"), + ) + + out = _unwrapped(capsys.readouterr().out) + assert f"error: {MARKUP_LIKE}" in out + if not quiet: + assert f"warning: {MARKUP_LIKE}" in out + + +def test_validate_bundle_issue_keeps_its_severity(monkeypatch, tmp_path, capsys): + """A bundle finding prints as ``[error] : ...``; Rich used to eat ``[error]``.""" + import fluid_build.forge.core.validators as validators + + issue = SimpleNamespace( + severity="error", + validator="manifest", + file="MANIFEST.json", + line=None, + column=None, + message=MARKUP_LIKE, + code="MANIFEST-TAMPER", + ) + report = SimpleNamespace( + issues=[issue], + status="fail", + bundle_digest="sha256:0", + summary={"total": 1, "error": 1}, + ) + monkeypatch.setattr(validators, "validate_bundle", lambda *_a, **_k: report) + monkeypatch.setattr(validate_mod, "_bundle_env_issue", lambda *_a: None) + args = SimpleNamespace(format="text", quiet=False, verbose=False, strict=False, report=None) + + validate_mod._run_bundle_validation( + tmp_path / "b.tgz", args, None, logging.getLogger("test"), 0.0 + ) + + out = _unwrapped(capsys.readouterr().out) + assert f"[error] manifest: MANIFEST.json: {MARKUP_LIKE}" in out + + +def test_validate_cli_error_context_and_suggestions_print_verbatim(monkeypatch, tmp_path, capsys): + """``fluid validate``'s own CLIError render: a context value and the suggestions.""" + import fluid_build._error_catalog as catalog + + def _load(*_a, **_k): + raise ValueError(MARKUP_LIKE) + + monkeypatch.setattr(validate_mod, "load_contract_with_overlay", _load) + monkeypatch.setattr( + catalog, + "enrich", + lambda *_a: ([f"pip install 'data-product-forge[gcp]' ({MARKUP_LIKE})"], None), + ) + path = _write(tmp_path, _contract("lakekeeper", lake_formation=True)) + + rc = cli_mod.main(["validate", path]) + + out = _unwrapped(capsys.readouterr().out) + assert rc == 1 + assert f"error: {MARKUP_LIKE}" in out + assert f"pip install 'data-product-forge[gcp]' ({MARKUP_LIKE})" in out + + +def test_validate_missing_file_names_the_whole_path(monkeypatch, capsys): + """The ``console_error`` lines wrap their text in ``[red]`` markup; the value is escaped.""" + import fluid_build.cli.security as security + from fluid_build.cli.core import FluidCLIError + + def _missing(*_a, **_k): + raise FluidCLIError(1, "file_not_found", "missing") + + monkeypatch.setattr(security, "validate_cli_path", _missing) + + rc = cli_mod.main(["validate", "orders[draft]\\"]) + + err = _unwrapped(capsys.readouterr().err) + assert rc == 1 + assert "Contract file not found: orders[draft]\\ " in err + " " + + +# --------------------------------------------------------------------------- +# main(): a CLIError's context, as `fluid generate iac` / `fluid apply` raise it +# --------------------------------------------------------------------------- + + +def test_generate_iac_unsupported_binding_names_the_expose(tmp_path, capsys): + """A real UnsupportedBindingError, raised by the AWS emitter, rendered by main().""" + path = _write(tmp_path, _contract("lakekeper", lake_formation=False)) + + rc = cli_mod.main(["generate", "iac", path]) + + out = capsys.readouterr().out + assert rc != 0 + assert "❌ unsupported_binding" in out + assert "error: exposes[orders] names Iceberg catalog 'lakekeper'" in _unwrapped(out) + + +def _raise_from_command(monkeypatch, err: Exception) -> None: + def _execute(self, args): + raise err + + monkeypatch.setattr(cli_mod.ProductionCLI, "_execute_command", _execute) + + +def _catalog_move_error(monkeypatch, workdir: str) -> CLIError: + """The CLIError ``fluid apply`` raises, built by the real guard and adapter.""" + import fluid_build.cli._apply_opentofu_engine as engine + from fluid_build.iac.providers.aws import AwsIacPlugin + + contract = { + "id": "analytics.lake", + "name": "Lake", + "exposes": [ + { + "exposeId": "orders", + "binding": { + "platform": "aws", + "format": "iceberg", + "location": { + "catalog": "lakekeeper", + "database": "sales", + "table": "orders", + "bucket": "lake", + "path": "orders/", + }, + }, + "contract": {"schema": [{"name": "order_id", "type": "string"}]}, + } + ], + } + state = [ + "aws_glue_catalog_database.analytics_lake_sales", + "aws_glue_catalog_table.analytics_lake_sales_orders", + ] + monkeypatch.setattr(engine.runner, "tofu_state_list", lambda *_a, **_k: state) + with pytest.raises(CLIError) as caught: + engine._guard_catalog_moves( + AwsIacPlugin(), contract, "aws", workdir, {}, logging.getLogger("test") + ) + return caught.value + + +def test_catalog_move_remediation_names_the_expose_and_keeps_commands_whole( + monkeypatch, tmp_path, capsys +): + # A long workdir pushes each `tofu -chdir= state rm
` past 80 columns. + workdir = str(tmp_path / ("runtime-" + "w" * 40) / "opentofu") + err = _catalog_move_error(monkeypatch, workdir) + assert err.event == "iceberg_catalog_move_blocked" + commands = err.context["remediation"] + assert commands and all(len(c) > 80 for c in commands) + _raise_from_command(monkeypatch, err) + + rc = cli_mod.main(["validate", "unused.fluid.yaml"]) + + out = capsys.readouterr().out + assert rc == 1 + assert "exposes[orders]: location.catalog lakekeeper" in out + lines = out.splitlines() + for command in commands: + # Whole on one printed line, so a copy-paste runs it with its address. + assert any(line.strip() == command for line in lines), command + + +def test_cli_error_message_context_and_suggestions_print_verbatim(monkeypatch, capsys): + err = CLIError( + 1, + f"event {MARKUP_LIKE}", + {"error": MARKUP_LIKE, "path": "C:\\work\\", "remediation": [MARKUP_LIKE]}, + ) + err.suggestions = [f"pip install 'data-product-forge[gcp]' ({MARKUP_LIKE})"] + _raise_from_command(monkeypatch, err) + + rc = cli_mod.main(["validate", "unused.fluid.yaml"]) + + out = capsys.readouterr().out + assert rc == 1 + assert f"❌ event {MARKUP_LIKE}" in out + assert f"[{err.error_slug}]" in out + assert f"error: {MARKUP_LIKE}" in out + # rich.markup.escape would have doubled this trailing backslash. + assert "path: C:\\work\\\n" in out + assert f"remediation: {[MARKUP_LIKE]!s}" in out + assert f"pip install 'data-product-forge[gcp]' ({MARKUP_LIKE})" in _unwrapped(out) + # The labels stay styled, not printed as literal markup. + assert "[dim]" not in out and "[red]" not in out + + +def test_fluid_cli_error_format_for_user_prints_brackets_literally(): + """``FluidCLIError.format_for_user`` (the top-level handler's renderer) had + the same flaw: ``exposes[orders]`` and ``pkg[gcp]`` lost their brackets.""" + import io + + from rich.console import Console + + from fluid_build.cli.core import FluidCLIError + + err = FluidCLIError( + 1, + "contract_invalid", + "exposes[orders] names [bold]nothing[/bold]", + context={"path": "C:\\work\\"}, + suggestions=["pip install 'data-product-forge[gcp]'"], + ) + buf = io.StringIO() + err.format_for_user(Console(file=buf, width=200, color_system=None)) + out = buf.getvalue() + assert "exposes[orders] names [bold]nothing[/bold]" in out + assert "data-product-forge[gcp]" in out + assert "C:\\\\work\\\\'" in out or "C:\\work\\" in out diff --git a/tests/iac/test_iac_catalog_move_wiring.py b/tests/iac/test_iac_catalog_move_wiring.py new file mode 100644 index 00000000..2d57a907 --- /dev/null +++ b/tests/iac/test_iac_catalog_move_wiring.py @@ -0,0 +1,265 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""``fluid apply`` stops before ``tofu plan`` when a catalog move would destroy. + +Behaviour through the real entrypoint, ``apply_via_opentofu``, with only the +``tofu`` shell-outs replaced: the state listing returns what an older +forge-cli applied for the contract, and ``tofu plan`` / ``tofu apply`` fail +the test if they are reached. The guard's own detection is covered in +``test_iac_catalog_moves.py``; this file pins that the engine runs it, for +the provider it applies, ahead of the plan. +""" + +from __future__ import annotations + +import argparse +import logging +from pathlib import Path +from typing import Any, Dict, List, Optional + +import pytest +import yaml + +from fluid_build.cli import _apply_opentofu_engine as engine +from fluid_build.cli._common import CLIError +from fluid_build.iac import runner + +pytestmark = [pytest.mark.unit, pytest.mark.provider] + +_LOG = logging.getLogger("test.iac.catalog_move_wiring") + +_SCHEMA = [{"name": "order_id", "type": "string"}] + +# What 0.19.0 applied for the contracts below, as ``tofu state list`` prints it. +_AWS_OLD_STATE = [ + "aws_glue_catalog_database.analytics_lake_sales", + "aws_glue_catalog_table.analytics_lake_sales_orders", + "aws_s3_bucket.analytics_lake_lake", +] +_SNOWFLAKE_OLD_STATE = [ + "snowflake_database.analytics_lake_ANALYTICS", + "snowflake_external_volume.analytics_lake_vol_FLUID_ANALYTICS_LAKE_VOL", + "snowflake_schema.analytics_lake_ANALYTICS_SALES", + "snowflake_table.analytics_lake_ANALYTICS_SALES_ORDERS", +] + + +def _aws_contract(catalog: Optional[str]) -> Dict[str, Any]: + location: Dict[str, Any] = { + "database": "sales", + "table": "orders", + "bucket": "lake", + "path": "orders/", + "region": "eu-west-1", + } + if catalog: + location.update(catalog=catalog, uri="http://lakekeeper:8181/catalog", warehouse="lake") + return { + "id": "analytics.lake", + "name": "Lake", + "exposes": [ + { + "exposeId": "orders", + "binding": {"platform": "aws", "format": "iceberg", "location": location}, + "contract": {"schema": list(_SCHEMA)}, + } + ], + } + + +def _snowflake_contract(catalog: Optional[str]) -> Dict[str, Any]: + location: Dict[str, Any] = { + "database": "ANALYTICS", + "schema": "SALES", + "table": "ORDERS", + "warehouse": "s3://lake/warehouse", + "iam_role_arn": "arn:aws:iam::123456789012:role/snowflake-lake", + } + if catalog: + location.update(catalog=catalog, uri="http://lakekeeper:8181/catalog") + return { + "id": "analytics.lake", + "name": "Lake", + "exposes": [ + { + "exposeId": "orders", + "binding": {"platform": "snowflake", "format": "iceberg", "location": location}, + "contract": {"schema": list(_SCHEMA)}, + } + ], + } + + +class _Tofu: + """The ``tofu`` shell-outs ``apply_via_opentofu`` makes, recorded.""" + + def __init__(self, monkeypatch, state: List[str], *, plan_allowed: bool) -> None: + self.calls: List[str] = [] + self.state = list(state) + self.plan_allowed = plan_allowed + ok = lambda command: runner.TofuResult(command, 0, "", "") # noqa: E731 + + def _unexpected(args, **_kw): + raise AssertionError(f"unexpected tofu {' '.join(args)}") + + def _record(name, result=None): + def _call(*_a, **_k): + self.calls.append(name) + return result() if callable(result) else result + + return _call + + def _plan(*_a, **_k): + self.calls.append("plan") + if not self.plan_allowed: + raise AssertionError("tofu plan ran although the catalog-move guard should block") + return runner.TofuResult("plan", 0, "", "", events=[]) + + def _apply(*_a, **_k): + raise AssertionError("tofu apply ran") + + monkeypatch.setattr(runner, "_run", _unexpected) + monkeypatch.setattr(runner, "tofu_path", lambda: "/usr/bin/tofu") + monkeypatch.setattr(runner, "require_tofu_version", lambda: None) + monkeypatch.setattr(runner, "tofu_init", _record("init", lambda: ok("init"))) + monkeypatch.setattr(runner, "tofu_state_list", _record("state-list", lambda: self.state)) + monkeypatch.setattr(runner, "tofu_state_resources", _record("state-resources", list)) + monkeypatch.setattr(runner, "tofu_prior_state_resources", _record("prior-state", list)) + monkeypatch.setattr(runner, "tofu_state_pull", _record("state-pull", lambda: ok("pull"))) + monkeypatch.setattr( + runner, "tofu_import", _record("import", runner.TofuResult("import", 1, "", "absent")) + ) + monkeypatch.setattr(runner, "tofu_plan", _plan) + monkeypatch.setattr(runner, "tofu_apply", _apply) + + +def _apply(monkeypatch, tmp_path: Path, contract: Dict[str, Any], *, dry_run: bool) -> int: + monkeypatch.delenv("FLUID_STATE_BACKEND", raising=False) + path = tmp_path / "contract.fluid.yaml" + path.write_text(yaml.safe_dump(contract), encoding="utf-8") + args = argparse.Namespace( + contract=str(path), + env=None, + provider=None, + workspace_dir=str(tmp_path), + state_backend=None, + dry_run=dry_run, + allow_data_loss=False, + ) + return engine.apply_via_opentofu(args, _LOG) + + +def _relocated(contract: Dict[str, Any], **changes: Any) -> Dict[str, Any]: + """``contract`` with its one expose's location changed (``None`` drops a key).""" + location = contract["exposes"][0]["binding"]["location"] + for key, value in changes.items(): + if value is None: + location.pop(key, None) + else: + location[key] = value + return contract + + +_BLOCKED = [ + pytest.param( + _aws_contract("lakekeeper"), + _AWS_OLD_STATE, + [_AWS_OLD_STATE[0], _AWS_OLD_STATE[1]], + id="aws-glue-database-and-table", + ), + # RT-707-1: a namespace-level expose (a database, no table); 0.19.0 created + # only the Glue database for it, so no Glue table can prove it. + pytest.param( + _relocated(_aws_contract("lakekeeper"), table=None), + [_AWS_OLD_STATE[0], _AWS_OLD_STATE[2]], + [_AWS_OLD_STATE[0]], + id="aws-table-less-glue-database", + ), + pytest.param( + _snowflake_contract("lakekeeper"), + _SNOWFLAKE_OLD_STATE, + [_SNOWFLAKE_OLD_STATE[1]], + id="snowflake-external-volume", + ), + # JRN-707-1: the same upgrade also changed the location, so today's + # emitter can no longer derive the volume the old release created. + pytest.param( + _relocated(_snowflake_contract("lakekeeper"), warehouse="analytics"), + _SNOWFLAKE_OLD_STATE, + [_SNOWFLAKE_OLD_STATE[1]], + id="snowflake-volume-warehouse-now-a-name", + ), + pytest.param( + _relocated(_snowflake_contract("lakekeeper"), iam_role_arn=None), + _SNOWFLAKE_OLD_STATE, + [_SNOWFLAKE_OLD_STATE[1]], + id="snowflake-volume-without-iam-role", + ), +] + + +@pytest.mark.parametrize("dry_run", [False, True], ids=["apply", "dry-run"]) +@pytest.mark.parametrize("contract, state, released", _BLOCKED) +def test_a_catalog_move_stops_the_apply_before_plan( + monkeypatch, tmp_path, contract, state, released, dry_run +): + tofu = _Tofu(monkeypatch, state, plan_allowed=False) + + with pytest.raises(CLIError) as excinfo: + _apply(monkeypatch, tmp_path, contract, dry_run=dry_run) + + assert excinfo.value.event == "iceberg_catalog_move_blocked" + assert "plan" not in tofu.calls + remediation = excinfo.value.context["remediation"] + assert [command.rsplit(" ", 1)[-1] for command in remediation] == released + assert all(" state rm " in command for command in remediation) + + +_PASSES = [ + pytest.param(_aws_contract(None), _AWS_OLD_STATE, id="aws-still-glue"), + pytest.param( + _aws_contract("lakekeeper"), + [a for a in _AWS_OLD_STATE if "glue" not in a], + id="aws-applied-by-this-release", + ), + pytest.param(_snowflake_contract(None), _SNOWFLAKE_OLD_STATE, id="snowflake-still-managed"), + pytest.param( + _snowflake_contract("lakekeeper"), + [a for a in _SNOWFLAKE_OLD_STATE if "external_volume" not in a], + id="snowflake-applied-by-this-release", + ), + pytest.param(_aws_contract("lakekeeper"), [], id="fresh-workdir"), +] + + +@pytest.mark.parametrize("contract, state", _PASSES) +def test_nothing_to_release_reaches_plan(monkeypatch, tmp_path, contract, state, caplog): + tofu = _Tofu(monkeypatch, state, plan_allowed=True) + + with caplog.at_level(logging.WARNING, logger=_LOG.name): + assert _apply(monkeypatch, tmp_path, contract, dry_run=True) == 0 + + assert "plan" in tofu.calls + assert "iceberg_catalog_move_probe_skipped" not in caplog.text + + +def test_the_blocked_event_links_to_the_guard_section(): + """The CLI prints the docs link the catalog maps the event to; unmapped, it + fell back to the generic troubleshooting page, which never mentions it.""" + from fluid_build._error_catalog import docs_url_for, suggestions_for + + url = docs_url_for("iceberg_catalog_move_blocked") + assert url is not None and url.endswith("cli/apply.html#iceberg-catalog-move-guard") + assert any("tofu state rm" in s for s in suggestions_for("iceberg_catalog_move_blocked")) diff --git a/tests/iac/test_iac_catalog_moves.py b/tests/iac/test_iac_catalog_moves.py index 2bc289d5..a4dce495 100644 --- a/tests/iac/test_iac_catalog_moves.py +++ b/tests/iac/test_iac_catalog_moves.py @@ -12,14 +12,17 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""The pre-plan guard for Glue resources of an Iceberg table that moved catalogs. +"""The pre-plan guard for resources of an Iceberg table that moved catalogs. A contract the previous release applied with ``catalog: lakekeeper`` (or rest, -polaris, nessie...) has a Glue database and table in its OpenTofu state, because -that release created them whatever the catalog said. The emitter no longer -does, so the next plan would DESTROY them, and destroying a Glue database -deletes every table in it. ``fluid apply`` fails closed before ``tofu plan`` -with the ``tofu state rm`` commands instead (``iac/catalog_moves.py``). +polaris, nessie...) on AWS has a Glue database and table in its OpenTofu state, +because that release created them whatever the catalog said; on Snowflake, +``lakekeeper`` / ``bigquery`` got an EXTERNAL VOLUME. The emitters no longer +create them, so the next plan would DESTROY them, and destroying a Glue +database deletes every table in it. ``fluid apply`` fails closed before +``tofu plan`` with the ``tofu state rm`` commands instead +(``iac/catalog_moves.py``). That the apply engine runs the guard is pinned +through ``apply_via_opentofu`` in ``test_iac_catalog_move_wiring.py``. """ from __future__ import annotations @@ -30,13 +33,18 @@ import pytest +from fluid_build.iac import runner from fluid_build.iac.catalog_moves import ( + AWS_GLUE_MOVES, + SNOWFLAKE_VOLUME_MOVES, CatalogMoveError, + catalog_move_spec, detect_catalog_moves, guard_catalog_moves, moved_iceberg_exposes, ) from fluid_build.iac.providers.aws import AwsIacPlugin +from fluid_build.iac.providers.snowflake import SnowflakeIacPlugin pytestmark = [pytest.mark.unit, pytest.mark.provider] @@ -132,6 +140,73 @@ def test_a_glue_database_a_parquet_expose_still_uses_is_not_flagged(self): assert detect_catalog_moves(AwsIacPlugin(), contract, state) == (ORDERS,) + def test_a_database_a_removed_parquet_expose_created_is_not_a_move(self): + """SEC-707-4: this release applied a Lakekeeper table and a parquet table + on one Glue database, then the parquet expose was removed. The database + is no longer declared, but the old release never created it for the + Iceberg expose (its Glue table is not in state), so destroying it is a + plain removal for the data-loss gate, not a catalog move.""" + contract = _contract(_expose("iceberg", "lakekeeper")) + state = [BUCKET, DB, FACTS] + + assert detect_catalog_moves(AwsIacPlugin(), contract, state) == () + guard_catalog_moves(AwsIacPlugin(), contract, state) + + def test_a_database_is_flagged_for_the_expose_whose_table_is_in_state(self): + """Two moved exposes on one database, only one applied by the old + release: the database and that table are flagged, and the message names + only that expose.""" + contract = _contract( + _expose("iceberg", "lakekeeper"), + _expose("iceberg", "rest", expose_id="facts", table="facts"), + ) + state = [BUCKET, DB, ORDERS] + + with pytest.raises(CatalogMoveError) as excinfo: + guard_catalog_moves(AwsIacPlugin(), contract, state) + + assert excinfo.value.addresses == (DB, ORDERS) + assert excinfo.value.exposes == (("orders", "lakekeeper"),) + + @pytest.mark.parametrize("catalog", ["lakekeeper", "rest", "polaris", "nessie", "bigquery"]) + def test_a_table_less_expose_s_database_is_flagged(self, catalog): + """RT-707-1: a namespace-level Iceberg expose names a database and no + table (a dynamic-routing sink's). The old release created the Glue + database for it and nothing else, so no Glue table can be in state to + prove it, and destroying the database is Glue ``DeleteDatabase``.""" + expose = _expose("iceberg", catalog) + del expose["binding"]["location"]["table"] + contract = _contract(expose) + state = [BUCKET, DB] + + assert detect_catalog_moves(AwsIacPlugin(), contract, state) == (DB,) + with pytest.raises(CatalogMoveError) as excinfo: + guard_catalog_moves(AwsIacPlugin(), contract, state) + assert excinfo.value.exposes == (("orders", catalog),) + + def test_a_table_less_expose_s_database_a_parquet_expose_uses_is_not_flagged(self): + """The database stays declared, so the plan does not destroy it.""" + expose = _expose("iceberg", "lakekeeper") + del expose["binding"]["location"]["table"] + contract = _contract(_expose("parquet", expose_id="facts", table="facts"), expose) + + assert detect_catalog_moves(AwsIacPlugin(), contract, [BUCKET, DB, FACTS]) == () + + def test_a_table_less_expose_does_not_vouch_for_another_s_database(self): + """The evidence rule still holds for an expose that declares a table: + a table-less sibling on another database flags only its own.""" + table_less = _expose("iceberg", "rest", expose_id="raw", table="raw") + table_less["binding"]["location"].update(database="raw") + del table_less["binding"]["location"]["table"] + contract = _contract(_expose("iceberg", "lakekeeper"), table_less) + raw_db = "aws_glue_catalog_database.analytics_lake_raw" + + with pytest.raises(CatalogMoveError) as excinfo: + guard_catalog_moves(AwsIacPlugin(), contract, [BUCKET, DB, FACTS, raw_db]) + + assert excinfo.value.addresses == (raw_db,) + assert excinfo.value.exposes == (("raw", "rest"),) + def test_a_module_prefixed_address_is_not_this_module_s(self): """``fluid`` writes a root module: a child module's resource is not one this emit change removes, so it is matched exactly, never by name.""" @@ -153,6 +228,151 @@ def test_the_caller_s_contract_is_not_mutated(self): assert contract == before +# --------------------------------------------------------------------------- +# Snowflake: the EXTERNAL VOLUME an old release created +# --------------------------------------------------------------------------- + +VOLUME = "snowflake_external_volume.analytics_lake_vol_FLUID_ANALYTICS_LAKE_VOL" +SF_TABLE = "snowflake_table.analytics_lake_ANALYTICS_SALES_ORDERS" + + +def _sf_expose( + catalog: Optional[str] = None, + *, + expose_id: str = "orders", + table: str = "ORDERS", + fmt: str = "iceberg", +) -> Dict[str, Any]: + location: Dict[str, Any] = { + "database": "ANALYTICS", + "schema": "SALES", + "table": table, + "warehouse": "s3://lake/warehouse", + "iam_role_arn": "arn:aws:iam::123456789012:role/snowflake-lake", + } + if catalog is not None: + location.update(catalog=catalog, uri="http://lakekeeper:8181/catalog") + return { + "exposeId": expose_id, + "binding": {"platform": "snowflake", "format": fmt, "location": location}, + "contract": {"schema": copy.deepcopy(SCHEMA)}, + } + + +class TestSnowflakeVolume: + """ARCH-1: 0.19.0 tested the raw catalog against a list that missed + ``lakekeeper``, ``bigquery`` and the ``iceberg-rest`` spelling, so those + exposes got an EXTERNAL VOLUME. The emitter now gives them none.""" + + @pytest.mark.parametrize( + "catalog, kind", + [ + ("lakekeeper", "lakekeeper"), + ("Lakekeeper", "lakekeeper"), + ("bigquery", "bigquery"), + ("iceberg-rest", "rest"), + ], + ) + def test_a_volume_the_old_release_created_is_flagged(self, catalog, kind): + contract = _contract(_sf_expose(catalog)) + + assert moved_iceberg_exposes(contract, spec=SNOWFLAKE_VOLUME_MOVES) == (("orders", kind),) + assert detect_catalog_moves(SnowflakeIacPlugin(), contract, [SF_TABLE, VOLUME]) == (VOLUME,) + + @pytest.mark.parametrize( + "catalog", [None, "snowflake", "rest", "iceberg_rest", "polaris", "unity", "nessie", "glue"] + ) + def test_a_catalog_whose_volume_did_not_change_is_not_a_move(self, catalog): + """No volume before and none now (rest, polaris...), or one both times + (Snowflake-managed): the old list and the kind table agree.""" + contract = _contract(_sf_expose(catalog)) + assert moved_iceberg_exposes(contract, spec=SNOWFLAKE_VOLUME_MOVES) == () + assert detect_catalog_moves(SnowflakeIacPlugin(), contract, [SF_TABLE, VOLUME]) == () + + def test_an_operator_named_volume_never_moves(self): + expose = _sf_expose("lakekeeper") + expose["binding"]["icebergConfig"] = {"properties": {"external_volume": "OPS_VOL"}} + assert moved_iceberg_exposes(_contract(expose), spec=SNOWFLAKE_VOLUME_MOVES) == () + + def test_a_volume_a_managed_expose_still_uses_is_not_flagged(self): + """The volume is named per contract, so a Snowflake-managed expose + alongside keeps it declared.""" + contract = _contract( + _sf_expose("lakekeeper"), _sf_expose(None, expose_id="facts", table="FACTS") + ) + assert detect_catalog_moves(SnowflakeIacPlugin(), contract, [SF_TABLE, VOLUME]) == () + + @pytest.mark.parametrize( + "changes", + [{"warehouse": "analytics"}, {"iam_role_arn": None}], + ids=["warehouse-now-a-catalog-name", "no-iam-role-arn"], + ) + def test_a_volume_is_flagged_after_an_upgrade_that_changed_the_location(self, changes): + """JRN-707-1: the before image re-runs today's emitter over today's + location, which derives no volume once the warehouse is a Lakekeeper + warehouse NAME or the role is gone. The volume the old release created + is keyed from the contract id alone, so it is still found.""" + expose = _sf_expose("lakekeeper") + location = expose["binding"]["location"] + for key, value in changes.items(): + if value is None: + del location[key] + else: + location[key] = value + contract = _contract(expose) + + assert detect_catalog_moves(SnowflakeIacPlugin(), contract, [SF_TABLE, VOLUME]) == (VOLUME,) + assert detect_catalog_moves(SnowflakeIacPlugin(), contract, [SF_TABLE]) == () + + @pytest.mark.parametrize( + "contract_id", ["analytics.lake", "gold.hr.employee_360_v1", "9-lives"] + ) + def test_the_derived_volume_address_is_the_one_the_emitter_writes(self, contract_id): + """The one address this module derives instead of emitting: pinned to + the Snowflake emitter's own key, so the two cannot drift apart.""" + contract = {**_contract(_sf_expose(None)), "id": contract_id} + binding = contract["exposes"][0]["binding"] + emitted = SnowflakeIacPlugin().emit(contract)["snowflake_external_volume"] + + assert set(SNOWFLAKE_VOLUME_MOVES.addresses_before(contract, binding)) == { + f"snowflake_external_volume.{key}" for key in emitted + } + + def test_raises_with_the_snowflake_remedy(self): + with pytest.raises(CatalogMoveError) as excinfo: + guard_catalog_moves( + SnowflakeIacPlugin(), + _contract(_sf_expose("lakekeeper")), + [SF_TABLE, VOLUME], + workdir="/w/snowflake/analytics.lake", + ) + + exc = excinfo.value + assert exc.kind == "iceberg-catalog-move" + assert exc.remediation == (f"tofu -chdir=/w/snowflake/analytics.lake state rm {VOLUME}",) + message = str(exc) + assert "1 Snowflake EXTERNAL VOLUME(s)" in message + assert "exposes[orders]: location.catalog lakekeeper" in message + assert "the resources stay in Snowflake" in message + assert "DROP EXTERNAL VOLUME" in message + + +class TestSpecLookup: + def test_the_in_tree_clouds(self): + assert catalog_move_spec(AwsIacPlugin()) is AWS_GLUE_MOVES + assert catalog_move_spec(SnowflakeIacPlugin()) is SNOWFLAKE_VOLUME_MOVES + assert catalog_move_spec(object(), "gcp") is None + + def test_a_plugin_s_own_hook_wins(self): + class _OutOfTree: + name = "aws" + + def catalog_move_spec(self): + return SNOWFLAKE_VOLUME_MOVES + + assert catalog_move_spec(_OutOfTree(), "aws") is SNOWFLAKE_VOLUME_MOVES + + # --------------------------------------------------------------------------- # the guard and its message # --------------------------------------------------------------------------- @@ -267,38 +487,110 @@ def test_another_provider_is_untouched(self, monkeypatch, engine): ) assert calls == [] - def test_an_empty_state_passes(self, monkeypatch, engine): + def _pull(self, monkeypatch, engine, result) -> None: + monkeypatch.setattr(engine.runner, "tofu_state_pull", lambda *_a, **_k: result) + + def test_an_empty_state_passes_quietly(self, monkeypatch, engine, caplog): + """A fresh workdir: ``state pull`` prints nothing, so there is nothing + to release and nothing to warn about.""" self._listing(monkeypatch, engine, []) - engine._guard_catalog_moves( - _SpyPlugin(), - _contract(_expose("iceberg", "lakekeeper")), - "aws", - "/w", - {}, - logging.getLogger("t"), - ) + self._pull(monkeypatch, engine, runner.TofuResult("state-pull", 0, "", "")) + plugin = _SpyPlugin() + + with caplog.at_level(logging.WARNING, logger="t"): + engine._guard_catalog_moves( + plugin, + _contract(_expose("iceberg", "lakekeeper")), + "aws", + "/w", + {}, + logging.getLogger("t"), + ) + + assert plugin.emits == 0 + assert caplog.records == [] + + @pytest.mark.parametrize( + "pulled, reason", + [ + ( + runner.TofuResult("state-pull", 1, "", "Error acquiring the state lock"), + "the state could not be read: `tofu state pull` failed: Error acquiring", + ), + ( + runner.TofuResult( + "state-pull", + 0, + '{"lineage": "l", "serial": 3, "resources": [{"type": "aws_s3_bucket"}]}', + "", + ), + "listed nothing, but the state holds 1 resource(s)", + ), + ], + ids=["unreadable", "listing-lost"], + ) + def test_a_failed_state_listing_is_a_warning(self, monkeypatch, engine, caplog, pulled, reason): + """SEC-707-6: ``tofu state list`` answers [] when it fails, which used + to skip the guard as if the workdir were fresh.""" + self._listing(monkeypatch, engine, []) + self._pull(monkeypatch, engine, pulled) + + with caplog.at_level(logging.WARNING, logger="t"): + engine._guard_catalog_moves( + _SpyPlugin(), + _contract(_expose("iceberg", "lakekeeper")), + "aws", + "/w", + {}, + logging.getLogger("t"), + ) + + (record,) = caplog.records + assert record.levelno == logging.WARNING + assert "iceberg_catalog_move_probe_skipped" in record.getMessage() + assert reason in record.getMessage() + assert "do NOT pass --allow-data-loss" in record.getMessage() + + def test_a_failed_probe_is_a_warning_not_a_failure(self, monkeypatch, engine, caplog): + """T4 / ARCH-6: the probe stays fail-open (the data-loss gate stands + behind it), but the operator is told, at WARNING, why it did not run.""" - def test_a_failed_probe_does_not_fail_the_apply(self, monkeypatch, engine): class _Broken(AwsIacPlugin): def emit(self, contract, actions=()): raise RuntimeError("probe exploded") self._listing(monkeypatch, engine, [DB, ORDERS]) - engine._guard_catalog_moves( - _Broken(), - _contract(_expose("iceberg", "lakekeeper")), - "aws", - "/w", - {}, - logging.getLogger("t"), - ) + with caplog.at_level(logging.WARNING, logger="t"): + engine._guard_catalog_moves( + _Broken(), + _contract(_expose("iceberg", "lakekeeper")), + "aws", + "/w", + {}, + logging.getLogger("t"), + ) + + (record,) = caplog.records + assert record.levelno == logging.WARNING + message = record.getMessage() + assert "iceberg_catalog_move_probe_skipped" in message + assert "the probe failed: RuntimeError: probe exploded" in message + assert '"expose": "orders"' in message and '"catalog": "lakekeeper"' in message + assert "do NOT pass --allow-data-loss" in message + + def test_snowflake_translates_to_the_same_cli_error(self, monkeypatch, engine): + from fluid_build.cli._common import CLIError - def test_the_engine_runs_it_after_the_packaging_guard_and_before_plan(self, engine): - import inspect + self._listing(monkeypatch, engine, [SF_TABLE, VOLUME]) + with pytest.raises(CLIError) as excinfo: + engine._guard_catalog_moves( + SnowflakeIacPlugin(), + _contract(_sf_expose("lakekeeper")), + "snowflake", + "/w", + {}, + logging.getLogger("t"), + ) - source = inspect.getsource(engine.apply_via_opentofu) - packaging_at = source.index("_guard_packaging_transitions(") - moves_at = source.index("_guard_catalog_moves(") - adopt_at = source.index("_adopt_existing(") - plan_at = source.index("runner.tofu_plan(") - assert packaging_at < moves_at < adopt_at < plan_at + assert excinfo.value.event == "iceberg_catalog_move_blocked" + assert excinfo.value.context["remediation"] == [f"tofu -chdir=/w state rm {VOLUME}"] diff --git a/tests/iac/test_iceberg_unknown_catalog_noise.py b/tests/iac/test_iceberg_unknown_catalog_noise.py new file mode 100644 index 00000000..101064ec --- /dev/null +++ b/tests/iac/test_iceberg_unknown_catalog_noise.py @@ -0,0 +1,116 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""A typo'd Iceberg catalog is reported once, as a typo. + +``catalog: lakekeper`` is refused (the answer decides whether a Glue table +exists), and that refusal is the whole story. Before, the same contract also +drew a Lake Formation refusal advising the author to "govern access in the +lakekeper catalog", and the native AWS planner logged the refusal as a +``plan_failed`` JSON line before the IaC raised it again. +""" + +from __future__ import annotations + +from typing import Any, Dict + +import pytest + +from fluid_build.iac.base import UnsupportedBindingError +from fluid_build.iac.catalog_moves import catalog_move_spec +from fluid_build.iac.governance_validation import validate_governance +from fluid_build.iac.iceberg_validation import validate_iceberg_bindings +from fluid_build.iac.providers.aws import AwsIacPlugin +from fluid_build.providers.base import ProviderError + +pytestmark = pytest.mark.unit + +ANALYST = "arn:aws:iam::123456789012:role/analyst" + + +def _contract(catalog: str, *, lake_formation: bool = True) -> Dict[str, Any]: + binding: Dict[str, Any] = { + "platform": "aws", + "format": "iceberg", + "location": { + "database": "sales", + "table": "orders", + "bucket": "lake", + "region": "us-east-1", + "catalog": catalog, + }, + } + if lake_formation: + binding["governance"] = { + "lakeFormation": { + "grants": [{"principal": ANALYST, "permissions": ["SELECT", "DESCRIBE"]}] + } + } + return { + "fluidVersion": "0.7.6", + "kind": "DataProduct", + "id": "gold.orders", + "name": "orders", + "metadata": {"layer": "Gold"}, + "exposes": [ + { + "exposeId": "orders", + "kind": "table", + "binding": binding, + "contract": {"schema": [{"name": "id", "type": "string"}]}, + } + ], + } + + +def test_validate_reports_the_typo_and_not_a_lake_formation_refusal(): + contract = _contract("lakekeper") + gov_errors, _ = validate_governance(contract) + ice_errors, _ = validate_iceberg_bindings(contract) + assert not any("lakekeper' catalog" in e for e in gov_errors), gov_errors + assert any("lakekeper" in e for e in ice_errors), ice_errors + + +def test_apply_refuses_the_typo_first(): + with pytest.raises(UnsupportedBindingError) as info: + AwsIacPlugin().emit(_contract("lakekeper")) + assert info.value.kind == "unknown-iceberg-catalog" + + +def test_a_real_external_catalog_still_refuses_lake_formation(): + with pytest.raises(UnsupportedBindingError) as info: + AwsIacPlugin().emit(_contract("lakekeeper")) + assert info.value.kind == "lake-formation-needs-glue-catalog" + + +def test_the_native_planner_refusal_is_not_logged_as_a_plan_failure(monkeypatch): + from fluid_build.providers.aws.provider import AwsProvider + + provider = AwsProvider(account_id="123456789012", region="us-east-1") + logged = [] + monkeypatch.setattr(provider, "err_kv", lambda **kw: logged.append(kw)) + with pytest.raises(ProviderError) as info: + provider.plan(_contract("lakekeper", lake_formation=False)) + assert "lakekeper" in str(info.value) + assert logged == [], logged + + +def test_only_a_real_catalog_move_spec_counts(): + class _Plugin: + name = "aws" + + def catalog_move_spec(self): + return {"not": "a spec"} + + assert catalog_move_spec(_Plugin()) is None diff --git a/tests/providers/aws/test_planner_unknown_iceberg_catalog.py b/tests/providers/aws/test_planner_unknown_iceberg_catalog.py new file mode 100644 index 00000000..debaf7d1 --- /dev/null +++ b/tests/providers/aws/test_planner_unknown_iceberg_catalog.py @@ -0,0 +1,95 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""The native AWS planner refuses an Iceberg catalog it does not know. + +An unknown ``location.catalog`` classifies as "not Glue", so ``catalog: glu`` +used to plan two S3 buckets and no Glue database or table, with no word, while +the AWS IaC refused the same value (``unknown-iceberg-catalog``). Whether a +Glue table exists hangs on the answer and nothing guarantees ``fluid +validate`` ran first, so the planner refuses it as the IaC does. +""" + +from __future__ import annotations + +import pytest + +from fluid_build.providers._iceberg_catalog import known_catalog_kinds +from fluid_build.providers.aws.plan.planner import plan_actions + +pytestmark = [pytest.mark.unit, pytest.mark.provider] + +ACCT = "123456789012" +REGION = "us-east-1" + + +def _contract(catalog, *, fmt="iceberg"): + location = {"database": "sales", "table": "orders", "bucket": "lake", "region": REGION} + if catalog is not None: + location["catalog"] = catalog + return { + "id": "gold.orders", + "name": "orders", + "exposes": [ + { + "exposeId": "orders", + "kind": "table", + "binding": {"platform": "aws", "format": fmt, "location": location}, + "contract": {"schema": [{"name": "id", "type": "string"}]}, + } + ], + } + + +def test_a_typo_is_refused_not_planned_without_glue(): + with pytest.raises(ValueError) as exc: + plan_actions(_contract("glu"), ACCT, REGION) + message = str(exc.value) + # The field, the value, the cause, the consequence and the remedy. + assert "exposes[orders]" in message + assert "'glu' (location.catalog)" in message + assert "does not know" in message + assert "no Glue database or table" in message + assert "Use one of: " + ", ".join(known_catalog_kinds()) + "." in message + assert "remove location.catalog" in message + + +def test_the_corrected_spelling_plans_the_glue_table(): + ops = [a["op"] for a in plan_actions(_contract("glue"), ACCT, REGION)] + assert "glue.ensure_database" in ops + assert "glue.ensure_iceberg_table" in ops + + +@pytest.mark.parametrize("spelling", ["Lakekeeper", "ICEBERG_REST", "iceberg-rest", "Snowflake"]) +def test_a_known_spelling_is_not_refused(spelling): + ops = [a["op"] for a in plan_actions(_contract(spelling), ACCT, REGION)] + assert "glue.ensure_iceberg_table" not in ops + + +def test_catalog_is_ignored_off_iceberg_as_the_iac_ignores_it(): + """``location.catalog`` means nothing to a plain Glue table format, so a + stray value there is not refused (the IaC's refusal is format-gated too).""" + ops = [a["op"] for a in plan_actions(_contract("glu", fmt="parquet"), ACCT, REGION)] + assert "glue.ensure_database" in ops + + +def test_the_provider_plan_surfaces_the_remedy(): + """``AwsProvider.plan`` wraps the planner's error text and drops the rest, + so the remedy has to travel in the message.""" + from fluid_build.providers.aws.provider import AwsProvider + from fluid_build.providers.base import ProviderError + + provider = AwsProvider(account_id=ACCT, region=REGION) + with pytest.raises(ProviderError, match=r"'glu'.*remove location\.catalog"): + provider.plan(_contract("glu")) diff --git a/tests/providers/test_iceberg_catalog_kinds.py b/tests/providers/test_iceberg_catalog_kinds.py index 937b35dd..ef0297a0 100644 --- a/tests/providers/test_iceberg_catalog_kinds.py +++ b/tests/providers/test_iceberg_catalog_kinds.py @@ -17,9 +17,21 @@ ``catalog: lakekeeper`` used to stream over REST while dbt's ``catalogs.yml`` wrote a Snowflake-managed table and the AWS IaC created a Glue table for the same name: four emitters each classified the free-string ``catalog`` by hand. -The unit tests pin the table; the agreement matrix runs EVERY emitter over -every kind and checks each answer against the kind's row, so a fifth -hand-rolled classification fails here rather than in a user's lake. +The unit tests pin the table; the agreement matrix runs these emitters over +every kind, an absent catalog and an unknown value, and checks each answer +against the kind's row: + +- dbt ``catalogs.yml`` on Snowflake and on BigQuery +- the Snowflake IaC, the AWS IaC and the Confluent (Tableflow) IaC +- the native AWS planner (``plan_actions``) +- ``fluid policy compile`` (``compile_policy``) +- the Kafka Connect sink validator and both sink derivers (Kafka Connect, + Debezium Server) +- ``fluid diff``'s Glue inspector + +A new hand-rolled classification in one of them fails here rather than in a +user's lake. ``fluid test``'s Glue check (``ContractValidator._iceberg_outside_glue``) +also reads the table and is not in the matrix. """ from __future__ import annotations @@ -378,3 +390,102 @@ def test_sink_validator_demands_exactly_what_the_row_requires(kind): for key in ("uri", "warehouse"): demanded = any(f"binding.location.{key}" in e for e in errors) assert demanded == (key in row.sink_requires), (key, errors) + + +@pytest.mark.parametrize("kind", MATRIX) +def test_aws_planner_plans_a_glue_table_only_for_glue(kind): + from fluid_build.providers.aws.plan.planner import plan_actions + + contract = _contract(_binding(kind, platform="aws", bucket="lake", region="us-east-1")) + if _row(kind, "aws").family == FAMILY_UNKNOWN: + # The planner's answer decides whether a Glue table exists, as the + # IaC's does: refused, never planned as "not Glue". + with pytest.raises(ValueError, match="does not know"): + plan_actions(contract, "123456789012", "us-east-1") + return + ops = [a["op"] for a in plan_actions(contract, "123456789012", "us-east-1")] + in_glue = _row(kind, "aws").family == FAMILY_GLUE + assert ("glue.ensure_iceberg_table" in ops) == in_glue + assert ("glue.ensure_database" in ops) == in_glue + # The bucket is the catalog's storage either way. + assert "s3.ensure_bucket" in ops + + +@pytest.mark.parametrize("kind", MATRIX) +def test_policy_compiler_grants_on_glue_only_for_glue(kind): + from fluid_build.policy.compiler import compile_policy + + contract = _contract(_binding(kind, platform="aws", bucket="lake", region="us-east-1")) + principal = "arn:aws:iam::123456789012:role/reader" + contract["accessPolicy"] = {"grants": [{"principal": principal, "permissions": ["read"]}]} + bindings, warnings = compile_policy(contract) + types = [b["resource_type"] for b in bindings] + row = _row(kind, "aws") + assert ("glue.table" in types) == (row.family == FAMILY_GLUE) + if row.family not in (FAMILY_GLUE, ic.FAMILY_SNOWFLAKE_MANAGED): + # A grant on a table outside Glue is reported, never compiled to + # silence; the unknown value is named so the reader can see it. + assert any(canonical_catalog_kind(kind) in w for w in warnings), warnings + + +@pytest.mark.parametrize("kind", MATRIX) +def test_confluent_iac_publishes_to_glue_only_for_glue(kind): + from fluid_build.iac.providers.confluent import ConfluentIacPlugin, validate_confluent_binding + + binding = _binding( + kind, + platform="confluent", + bucket="lake", + environment_id="env-1", + kafka_cluster_id="lkc-1", + confluent_role_arn="arn:aws:iam::123456789012:role/tableflow", + topic="orders", + ) + contract = _contract(binding) + res = ConfluentIacPlugin().emit(contract) + errors, _ = validate_confluent_binding(contract) + # Tableflow publishes to AWS Glue, so an absent catalog is Glue here, as + # on AWS: the AWS row is the one to read. + in_glue = _row(kind, "aws").family == FAMILY_GLUE + assert bool(res.get("confluent_catalog_integration")) == in_glue + # The validator refuses exactly what the emitter declines to publish. + assert any("binding.location.catalog" in e for e in errors) == (not in_glue), errors + assert res.get("confluent_tableflow_topic") + + +@pytest.mark.parametrize("kind", MATRIX) +def test_dbt_bigquery_writes_biglake_only_for_biglake(kind): + from fluid_build.engines.dbt.catalogs_yml import generate_catalogs_yml + + build = {"engine": "dbt", "execution": {"runtime": {"platform": "gcp"}}} + content = generate_catalogs_yml(_contract(_binding(kind, platform="gcp", bucket="lake")), build) + # ``biglake_metastore`` is dbt-bigquery's only catalog type, and an absent + # catalog has always meant BigLake there (the GCP row's REST default is + # the streaming sink's, not dbt's: see ``default_catalog_kind``). + biglake = kind is None or catalog_kind_info(kind).name == "bigquery" + if not biglake: + assert content is None + return + assert content is not None + integration = yaml.safe_load(content)["catalogs"][0]["write_integrations"][0] + assert integration["catalog_type"] == "biglake_metastore" + + +@pytest.mark.parametrize("kind", MATRIX) +def test_diff_inspects_glue_only_for_glue(kind, monkeypatch, tmp_path): + from fluid_build.cli import _diff_live + from fluid_build.providers import aws_validation + + # Stop at the first AWS call: what is asserted is whether Glue would be + # read at all, never a live answer. + monkeypatch.setattr(aws_validation, "BOTO3_AVAILABLE", False) + contract = _contract(_binding(kind, platform="aws", bucket="lake", region="us-east-1")) + (result,) = _diff_live.compare_live(contract, tmp_path).exposes + if _row(kind, "aws").family == FAMILY_GLUE: + assert result.target.startswith("glue:sales.orders"), result + assert result.status == _diff_live.ERROR and "boto3" in (result.detail or "") + else: + # A Glue table of that name would be someone else's: neither "absent" + # nor a comparison with it would be true. + assert result.status == _diff_live.NOT_CHECKED + assert f"Iceberg catalog {canonical_catalog_kind(kind)}" in (result.detail or "")