From 9a58fca2b80202cfce02ef0ff22fb29832b4cf53 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 23:31:22 +0100 Subject: [PATCH 1/7] feat(databricks): one Genie space for a product deployed from several identities extensions.databricksGenie.parentPath names a workspace folder for the product's Genie space, so a laptop and CI keep one space instead of one each in their own folders. The space is created there (parent_path), and only a space directly in that folder is adopted. One an earlier apply made in the identity's own folder is moved in (PATCH parent_path keeps its id and conversations), and a copy left there is trashed. A space inherits its folder's permissions, so the folder is the trust boundary. fluid validate refuses anything under /Shared, where the users group holds CAN_MANAGE and every folder in it inherits that. The apply stops before tofu apply on a folder that is missing, is not a folder, has permissions it cannot read, or gives users CAN_EDIT or CAN_MANAGE. Measured on a live workspace (2026-11-12): - A create accepts /Workspace/... and a trailing slash; GET returns the bare form. - A missing folder is 404 and a relative path 400; paths are case-sensitive. - PATCH {parent_path} moves a space. The /Workspace normalisation follows terraform-provider-databricks' common.WorkspacePathPrefixDiffSuppress, but strips the prefix only as a whole segment. --- AGENTS.md | 14 +- CHANGELOG.md | 18 +- fluid_build/iac/providers/databricks.py | 4 +- fluid_build/iac/providers/databricks_genie.py | 407 +++++++++++++++--- tests/iac/_fake_databricks.py | 180 ++++++-- tests/iac/test_iac_databricks_genie.py | 256 ++++++++++- .../iac/test_iac_databricks_genie_fake_e2e.py | 24 ++ 7 files changed, 809 insertions(+), 94 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 64b2a500..161c4f87 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -354,7 +354,7 @@ example questions in natural language: ```yaml extensions: - databricksGenie: {} # or {title: "Site 360"} + databricksGenie: {} # or {title: "Site 360", parentPath: /genie/asset} ``` `fluid apply` creates the space after the tables, masks and row filters, and keeps it in step: @@ -366,7 +366,17 @@ under their Unity Catalog grants. Prompt matching is never turned on. Only the p `SELECT` get `CAN RUN`. An `agentPolicy` that denies `qa` or `analysis`, or allows neither, refuses the space. The space is found again by the marker `[fluid-genie:]` at the end of its description, only in the deploying identity's own folder (one anywhere else is left alone), and removing the block trashes -it. Code: `fluid_build/iac/providers/databricks_genie.py`. +it. + +A product deployed from more than one identity (a laptop and CI) sets `parentPath`: a workspace +folder that a workspace admin creates, with `CAN_MANAGE` only for those identities. The space is +made there and adopted only there, so the identities keep one space. One an earlier apply made +in the identity's own folder is moved in, keeping its conversations. A space inherits its folder's +permissions, so the folder is the trust boundary. `/Shared` is refused offline, because every +workspace user holds `CAN_MANAGE` there. The apply also stops, before `tofu`, on a folder that +does not exist or one where the `users` group can edit or manage. Removing the block leaves a +space in `parentPath` with a note naming it. +Code: `fluid_build/iac/providers/databricks_genie.py`. --- diff --git a/CHANGELOG.md b/CHANGELOG.md index d3bd9927..fe222fa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,12 +24,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 finds the space by a `[fluid-genie:]` marker at the end of its description, in the deploying identity's own folder only (a space carrying the marker anywhere else is left alone, with a note), and updates it with its `etag`. A second apply changes nothing, and removing the - block trashes the space. Deploy a product from one identity (CI, say): a space another - identity made is left alone, and that identity makes its own. + block trashes the space. Deploy a product from one identity (CI, say), or name a shared folder + with `parentPath` (below): a space another identity made in its own folder is left alone. `fluid apply --mode dry-run` prints what would change. The space is made through the Genie REST API (the `[databricks]` extra), since the databricks/databricks provider has no Genie space resource. One space per product; a space over several products is a follow-up. +- **One Genie space for a product deployed from more than one identity.** + `extensions.databricksGenie.parentPath` names a workspace folder, such as `/genie/`. + A workspace admin creates it and gives `CAN_MANAGE` only to the identities that deploy the + product. `fluid apply` creates the space there and adopts only a space directly in that + folder, so a laptop and CI keep one space instead of one each. A space an earlier apply made + in the identity's own folder is moved in and keeps its conversations. A copy left there + afterwards is trashed. A space inherits its folder's permissions, so the folder is the trust + boundary. `fluid validate` refuses anything under `/Shared`: every workspace user holds + `CAN_MANAGE` there (measured), so each could read every reader's conversations. `fluid apply` + stops before `tofu apply` when the folder is missing or not a folder, when its permissions + cannot be read, or when the `users` group holds `CAN_EDIT` or `CAN_MANAGE` on it. Removing the + whole block leaves a space in `parentPath` in place, with a note naming it: without the block, + the apply no longer knows which folder to trust. + - **Databricks: Unity Catalog primary and foreign keys from the contract.** A column tagged `primary-key` (all of them, in schema order, for a composite key; each must be `required: true`) becomes the table's PRIMARY KEY, and one tagged `foreign-key` with diff --git a/fluid_build/iac/providers/databricks.py b/fluid_build/iac/providers/databricks.py index 34f1d1e0..2867ba78 100644 --- a/fluid_build/iac/providers/databricks.py +++ b/fluid_build/iac/providers/databricks.py @@ -110,7 +110,9 @@ ``agentPolicy`` as instructions. The provider has no Genie space resource, so :meth:`DatabricksIacPlugin.converge_output_ports` creates, updates or trashes it through the Genie API after the masks and row filters are in place, and finds it again by a marker in its - description (``databricks_genie.py``). + description (``databricks_genie.py``): in the deploying identity's own folder, or in the + workspace folder ``extensions.databricksGenie.parentPath`` names, which a product deployed + from more than one identity shares. * ``binding.location.region`` is the region the workspace is in, declared so a contract's ``sovereignty`` policy can check it (``policy/sovereignty.py`` diff --git a/fluid_build/iac/providers/databricks_genie.py b/fluid_build/iac/providers/databricks_genie.py index 4a35dcf0..34f77c02 100644 --- a/fluid_build/iac/providers/databricks_genie.py +++ b/fluid_build/iac/providers/databricks_genie.py @@ -4,9 +4,9 @@ """A data product's Databricks AI/BI Genie space: compiled from its contract, kept in step by ``fluid apply``. -A contract opts in with ``extensions.databricksGenie`` (``{}``, or ``{title: ...}``). The product -gets one space holding its Databricks tables and any metric view over them, where people ask -the product's own example questions in natural language. +A contract opts in with ``extensions.databricksGenie`` (``{}``, or ``{title: ..., parentPath: +...}``). The product gets one space holding its Databricks tables and any metric view over them, +where people ask the product's own example questions in natural language. What the space holds (``serialized_space`` version 2, the body ``POST /api/2.0/genie/spaces`` takes; field rules from databricks/databricks-agent-skills ``databricks-genie-agents`` @@ -52,6 +52,16 @@ someone plants is never filled, shared to the readers or trashed. No local record, so a fresh CI runner finds the same space; two spaces carrying one product's marker stop the apply. +Deployed from more than one identity (a laptop and CI), each would make its own space in its +own folder, so ``parentPath`` names one workspace folder for the space instead. A space inherits +its folder's permissions (measured), so the folder's permissions become the trust boundary: +whoever can edit or manage the folder can edit the space and make one there carrying the marker. +So the apply adopts only a space directly in that folder, and refuses a folder in which the +``users`` group (every workspace user) holds ``CAN_EDIT`` or ``CAN_MANAGE``: all of ``/Shared``, +which grants it ``CAN_MANAGE``, refused offline. A space an earlier apply made in the identity's +own folder is moved into the named one (``PATCH parent_path``, which keeps its conversations), and +a copy left in it once the product's space is there is trashed. + Measured on a live workspace (2026-11-12) and handled: the service keeps a space as sent except that it leaves out a ``false`` flag (never sent, so a second apply compares equal); a new space shows in the list some seconds after it is made (a create waits for it, or an apply straight @@ -93,10 +103,19 @@ #: The contract's opt-in: ``extensions.databricksGenie``. EXTENSION_KEY = "databricksGenie" -_BLOCK_KEYS = frozenset({"title"}) +_BLOCK_KEYS = frozenset({"title", "parentPath"}) SPACES_PATH = "/api/2.0/genie/spaces" PERMISSIONS_PATH = "/api/2.0/permissions/genie/{}" +WORKSPACE_STATUS_PATH = "/api/2.0/workspace/get-status" +FOLDER_PERMISSIONS_PATH = "/api/2.0/permissions/directories/{}" +#: The group every workspace user is in, as the Permissions API names it. +ALL_USERS = "users" +#: The folder that grants :data:`ALL_USERS` ``CAN_MANAGE`` directly; every folder and space under +#: it inherits that (measured on a live workspace, 2026-11-12), and a workspace permission +#: cannot be taken away below the folder that grants it ("objects in a folder inherit all +#: permissions settings of that folder", Databricks access control docs). +SHARED_FOLDER = "/Shared" TABLES_PATH = "/api/2.1/unity-catalog/tables" WAREHOUSE_ENV = "DATABRICKS_WAREHOUSE_ID" DATABRICKS_EXTRA_INSTALL = "pip install 'data-product-forge[databricks]'" @@ -166,11 +185,20 @@ class Space: readers: Tuple[str, ...] #: ``serialized_space``, without the metric views (those are read at apply time). serialized: Dict[str, Any] + #: The workspace folder the space lives in (``parentPath``, as :func:`workspace_folder` + #: writes it), or ``""``: the deploying identity's own folder, where the API puts a space + #: when the create names none. + parent_path: str = "" @property def marker(self) -> str: return marker(self.product_id) + @property + def folder(self) -> str: + """Where the space lives, for a message.""" + return self.parent_path or "this identity's folder" + @dataclass class Compiled: @@ -211,6 +239,7 @@ def compile_space( if not requested(contract): return out title = _block_title(contract, out) + parent_path = _block_parent_path(contract, out) out.refusals += _table_refusals(tables) if out.refusals: return out @@ -235,6 +264,7 @@ def compile_space( tables=tuple(t.full_name for t in ordered), readers=_readers(tables, grants), serialized=serialized, + parent_path=parent_path, ) return out @@ -289,6 +319,69 @@ def _block_title(contract: Mapping[str, Any], out: Compiled) -> str: return "" +def workspace_folder(path: str) -> str: + """``path`` as the Genie API reads a space's ``parent_path`` back: without the + ``/Workspace`` prefix a create accepts and without a trailing ``/`` (both measured, + 2026-11-12). terraform-provider-databricks compares a dashboard's ``parent_path`` the same + way (``common.WorkspacePathPrefixDiffSuppress``); here the prefix counts only as a whole + segment. Workspace paths are case-sensitive (``/shared/x`` is not ``/Shared/x``: 404, + measured), so nothing is folded, and nothing is trimmed: a folder named ``"/x "`` is not + ``/x``.""" + path = str(path or "") + if path == "/Workspace" or path.startswith("/Workspace/"): + path = path[len("/Workspace") :] + return path.rstrip("/") + + +def _block_parent_path(contract: Mapping[str, Any], out: Compiled) -> str: + """The folder ``parentPath`` names, as :func:`workspace_folder` writes it, or ``""`` without + one; what is wrong with it goes into ``out.refusals``.""" + block = contract["extensions"][EXTENSION_KEY] + if not isinstance(block, Mapping) or "parentPath" not in block: + return "" + raw = block["parentPath"] + where = f"{_WHERE}.parentPath" + folder = workspace_folder(raw.strip()) if isinstance(raw, str) else "" + if not isinstance(raw, str) or not raw.strip(): + problem = "not a folder path" + elif any(ord(ch) < 32 or ord(ch) == 127 for ch in raw): + problem = "a path with a control character in it" + elif not raw.strip().startswith("/"): + problem = "not absolute, and Genie takes a folder from the workspace root" + elif not folder: + problem = "the workspace root, and the space goes in a folder" + elif any(part in ("", ".", "..") for part in folder.split("/")[1:]): + problem = "a path with an empty, '.' or '..' part" + else: + problem = "" + if problem: + out.refusals.append( + _refusal( + "invalid", + f"{where} is {raw!r}: {problem}.", + f"Set {where} to a workspace folder such as /genie/, or leave it out for " + "the deploying identity's own folder.", + ) + ) + return "" + if folder == SHARED_FOLDER or folder.startswith(SHARED_FOLDER + "/"): + out.refusals.append( + _refusal( + "folder", + f"{where} is {raw!r}, in {SHARED_FOLDER}, where every workspace user holds " + f"CAN_MANAGE (the {ALL_USERS} group, granted on {SHARED_FOLDER} itself and " + "inherited by every folder in it). Each of them could then edit the Genie space's " + "instructions, read every reader's conversations, and make a space there that the " + "apply takes for the product's.", + f"Use a folder outside {SHARED_FOLDER} that a workspace admin creates (/genie/ " + "at the workspace root, say, which only admins can write) and gives CAN_MANAGE only " + "to the identities that deploy the product.", + ) + ) + return "" + return folder + + def _table_refusals(tables: Sequence[Any]) -> List[UnsupportedBindingError]: """A space needs one to :data:`MAX_TABLES` tables, each one an ``agentPolicy`` allows.""" if not tables: @@ -791,30 +884,189 @@ def converge( ) client = _client(client_factory) say("\n Databricks Genie space (Genie API, not OpenTofu):") + if space.parent_path: + _check_folder(client, space.parent_path, say) found = _call(client, "list the Genie spaces", lambda: _find(client, space.marker)) - found = _call(client, "check whose Genie spaces those are", lambda: _owned(client, found, say)) - if len(found) > 1: - raise GenieError( - f"{len(found)} Genie spaces carry {space.marker} in their description: " - + ", ".join(str(s.get("space_id")) for s in found) - + ". fluid apply keeps one space per product and cannot tell which is the product's.", - ("Trash the extra spaces (or remove the marker from their description), then re-run.",), - ) + found, own = _call( + client, + "check whose Genie spaces those are", + lambda: _placed(client, found, say, space.parent_path), + ) + # One space per product. A copy in this identity's own folder is moved into parentPath + # when the folder has none yet (one copy only: which one would be a guess), and trashed + # once the product's space is there. + _at_most_one(found or own, space) serialized = _with_metric_views(client, space) - if found: - space_id = str(found[0]["space_id"]) + space_id = str(found[0]["space_id"]) if found else "" + if not found and own: + space_id = str(own.pop()["space_id"]) + _move(client, space_id, space, say, dry_run) + if space_id: _update_found(client, space_id, space, serialized, say, dry_run) elif dry_run: - say(f" would create {space.title!r}: {_summary(serialized, space.tables)}") + say(f" would create {space.title!r}{_in(space)}: {_summary(serialized, space.tables)}") if space.readers: say(f" would share it ({READER_LEVEL}) with {', '.join(space.readers)}") - say(" dry-run: no Genie space was created, changed or shared.") + say(_DRY_RUN) return else: space_id = _create_new(client, space, serialized, say) _share(client, space_id, space.readers, say, dry_run) + for copy in own: + _trash_copy(client, copy, space, say, dry_run) if dry_run: - say(" dry-run: no Genie space was created, changed or shared.") + say(_DRY_RUN) + + +_DRY_RUN = " dry-run: no Genie space was created, changed, moved, shared or trashed." + + +def _in(space: Space) -> str: + """`` in `` for a space ``parentPath`` places, else nothing (its own folder).""" + return f" in {space.parent_path}" if space.parent_path else "" + + +def _at_most_one(spaces: Sequence[Mapping[str, Any]], space: Space) -> None: + if len(spaces) > 1: + raise GenieError( + f"{len(spaces)} Genie spaces carry {space.marker} in their description: " + + ", ".join(str(s.get("space_id")) for s in spaces) + + ". fluid apply keeps one space per product and cannot tell which is the product's.", + ("Trash the extra spaces (or remove the marker from their description), then re-run.",), + ) + + +def _check_folder(client: Any, folder: str, say: Say) -> None: + """Stop unless ``folder`` is a workspace folder on which the :data:`ALL_USERS` group (every + workspace user) holds neither ``CAN_EDIT`` nor ``CAN_MANAGE``, directly or inherited. + + A space inherits every permission on its folder (measured, 2026-11-12): whoever can edit + the folder can edit the space's instructions, and whoever manages it can also read every + reader's conversations and make a space there carrying the product's marker, which the + apply then adopts. The folder's permissions are the trust boundary, and one every user + holds is none. Permissions this identity cannot read stop the apply too: they could be + those. + """ + object_id = _folder_object_id(client, folder) + acl = _call( + client, + f"read the permissions on the folder {folder}", + lambda: client.api_client.do("GET", FOLDER_PERMISSIONS_PATH.format(object_id)), + ) + held = sorted(_group_levels(acl, ALL_USERS), key=lambda level: _LEVEL_RANK.get(level, 0)) + if held and held[-1] in _KEPT_LEVELS: + harm = "edit the space's instructions" + if held[-1] == "CAN_MANAGE": + harm = ( + "edit the space's instructions, read every reader's conversations and make a " + "space there that the apply would take for the product's" + ) + raise GenieError( + f"every workspace user holds {held[-1]} on the folder {folder} (the {ALL_USERS} " + f"group), and a Genie space inherits its folder's permissions: each of them could " + f"{harm}.", + ( + f"Take CAN_EDIT and CAN_MANAGE on {folder} away from {ALL_USERS}, or set " + f"{_WHERE}.parentPath to a folder that only the identities deploying the product " + f"can edit or manage (not under {SHARED_FOLDER}, which grants {ALL_USERS} " + "CAN_MANAGE).", + ), + ) + if held: + say( + f" note: every workspace user holds {held[-1]} on {folder}, which the space " + "inherits: they can open it, and each answer still follows the asker's own Unity " + "Catalog grants." + ) + + +def _folder_object_id(client: Any, folder: str) -> str: + """The object id of the folder ``parentPath`` names, for the Permissions API path; a folder + that does not exist, or a path that is no folder, stops the apply with what to do.""" + where = f"{_WHERE}.parentPath" + try: + status = client.api_client.do("GET", WORKSPACE_STATUS_PATH, query={"path": folder}) or {} + except Exception as exc: # noqa: BLE001 — a missing folder is said as such + if _status(exc) != 404: + raise GenieError( + f"could not read the folder {folder}: {type(exc).__name__}: {_scrub(exc)}", + (_remediation(_status(exc), f"read the folder {folder}"),), + ) from exc + raise GenieError( + f"{where} names {folder}, and the workspace has no such folder (Genie makes a space " + "only in a folder that exists).", + ( + f"A workspace admin creates {folder} (outside {SHARED_FOLDER}) and gives CAN_MANAGE " + "on it only to the identities that deploy the product; then re-run fluid apply.", + ), + ) from exc + kind = str(status.get("object_type") or "") if isinstance(status, Mapping) else "" + if kind.upper() != "DIRECTORY": + raise GenieError( + f"{where} names {folder}, which is " + + (f"a {kind.lower()}" if kind else "not a folder") + + ", and a Genie space goes in a folder.", + (f"Set {where} to a workspace folder.",), + ) + return quote(str(status.get("object_id"))) + + +def _group_levels(doc: Any, group: str) -> set: + """Every level ``group`` holds in a Permissions API document, direct or inherited.""" + levels: set = set() + entries = doc.get("access_control_list") if isinstance(doc, Mapping) else None + for entry in entries or (): + if isinstance(entry, Mapping) and entry.get("group_name") == group: + for held in entry.get("all_permissions") or (): + if isinstance(held, Mapping) and held.get("permission_level"): + levels.add(str(held["permission_level"])) + return levels + + +def _move(client: Any, space_id: str, space: Space, say: Say, dry_run: bool) -> None: + """Move a space from this identity's own folder into ``parentPath`` (``PATCH parent_path``, + measured 2026-11-12), so its id, link and conversations stay.""" + if dry_run: + say( + f" would move space {space_id} from this identity's own folder into {space.parent_path}" + ) + return + _call( + client, + f"move the Genie space into {space.parent_path}", + lambda: client.api_client.do( + "PATCH", f"{SPACES_PATH}/{quote(space_id)}", body={"parent_path": space.parent_path} + ), + ) + parent = _call(client, "read the moved Genie space", lambda: _parent(client, space_id)) + if workspace_folder(parent) != space.parent_path: + raise GenieError( + f"the Genie API accepted the move of space {space_id} into {space.parent_path}, and " + f"the space is in {parent or 'a folder this identity cannot read'}.", + (f"Move it into {space.parent_path} by hand, or trash it, then re-run fluid apply.",), + ) + say(f" moved space {space_id} from this identity's own folder into {space.parent_path}") + + +def _trash_copy( + client: Any, copy: Mapping[str, Any], space: Space, say: Say, dry_run: bool +) -> None: + """Trash a space carrying the marker in this identity's own folder, now that the product's + space is the one in ``parentPath``.""" + space_id = str(copy.get("space_id")) + what = ( + f"{copy.get('title')!r} ({space_id}), a copy in this identity's own folder: the product's " + f"space is the one{_in(space)}" + ) + if dry_run: + say(f" would trash {what}") + return + _call( + client, + "trash the Genie space", + lambda: client.api_client.do("DELETE", f"{SPACES_PATH}/{quote(space_id)}"), + ) + say(f" trashed {what}") def _with_metric_views(client: Any, space: Space) -> Dict[str, Any]: @@ -849,7 +1101,7 @@ def _update_found( dry_run: bool, ) -> None: live = _call(client, "read the Genie space", lambda: _get(client, space_id)) - title = _title_kept(live.get("title"), space.title, say) + title = _title_kept(live.get("title"), space, say) changes = differences(live, space, serialized) if not changes: say(f" {space.title!r}: no change ({_url(space_id)})") @@ -869,19 +1121,30 @@ def _create_new(client: Any, space: Space, serialized: Mapping[str, Any], say: S space_id = str(created.get("space_id") or "") if not space_id: raise GenieError("the Genie API created a space and returned no space_id.") - say(f" created {space.title!r}: {_summary(serialized, space.tables)} ({_url(space_id)})") - _title_kept(created.get("title"), space.title, say) + say( + f" created {space.title!r}{_in(space)}: {_summary(serialized, space.tables)} " + f"({_url(space_id)})" + ) + _title_kept(created.get("title"), space, say) _wait_listed(client, space, space_id, say) - _found_again(client, space_id) + _found_again(client, space, space_id) return space_id -def _found_again(client: Any, space_id: str) -> None: +def _found_again(client: Any, space: Space, space_id: str) -> None: """Stop, and trash the new space, when it is not where the next apply looks for it: a space every apply could not find would be made again on each one.""" - home = _call(client, "read which identity this run is", lambda: _home(client)) + home = "" + if not space.parent_path: + home = _call(client, "read which identity this run is", lambda: _home(client)) + where = space.parent_path or f"this identity's own folder ({home})" parent = _call(client, "read the new Genie space", lambda: _parent(client, space_id)) - if _in_folder(parent, home): + landed = ( + workspace_folder(parent) == space.parent_path + if space.parent_path + else _in_folder(parent, home) + ) + if landed: return try: _call( @@ -892,17 +1155,26 @@ def _found_again(client: Any, space_id: str) -> None: except GenieError as exc: raise GenieError( f"the Genie API put the new space {space_id} in " - f"{parent or 'a folder this identity cannot read'}, not in this identity's own folder " - f"({home}), where the next apply looks for it, and trashing it failed: {exc}", + f"{parent or 'a folder this identity cannot read'}, not in {where}, where the next " + f"apply looks for it, and trashing it failed: {exc}", (f"Trash space {space_id} by hand before the next apply, which would make another.",), ) from exc raise GenieError( f"the Genie API put the new space in {parent or 'a folder this identity cannot read'}, " - f"not in this identity's own folder ({home}), where the next apply looks for it, so it " - "was trashed rather than made again on every apply.", + f"not in {where}, where the next apply looks for it, so it was trashed rather than made " + "again on every apply.", ( - "Deploy with an identity whose home folder is /Users/, or report this " - "to forge-cli with the folder named above.", + ( + ( + f"Check that {space.parent_path} is a folder this identity can read, or report " + "this to forge-cli with the folder named above." + ) + if space.parent_path + else ( + "Deploy with an identity whose home folder is /Users/, or report " + "this to forge-cli with the folder named above." + ) + ), ), ) @@ -927,14 +1199,15 @@ def _wait_listed(client: Any, space: Space, space_id: str, say: Say) -> None: time.sleep(_POLL_SECONDS) -def _title_kept(live: Any, wanted: str, say: Say) -> str: - """The title to send: ``wanted``, or the live one when the service suffixed it because - another space in the folder has ``wanted`` (sending ``wanted`` again is 409), said as +def _title_kept(live: Any, space: Space, say: Say) -> str: + """The title to send: the contract's, or the live one when the service suffixed it because + another space in the folder has the contract's (sending that again is 409), said as such.""" live = str(live or "") + wanted = space.title if live != wanted and _same_title(live, wanted): say( - f" note: the space is titled {live!r}: another space in this identity's folder is " + f" note: the space is titled {live!r}: another space in {space.folder} is " f"already called {wanted!r} and Genie keeps titles unique there. Give this one a " f"title of its own with extensions.{EXTENSION_KEY}.title." ) @@ -951,9 +1224,12 @@ def _same_title(live: Any, wanted: str) -> bool: def preflight(compiled: Compiled, client_factory: Optional[ClientFactory] = None) -> None: """Before ``tofu apply``: the client a space the contract asks for needs (the SDK extra and - the credential), so a missing one stops the run before anything is applied.""" + the credential), and the folder ``parentPath`` names (:func:`_check_folder`), so a missing + one, or a folder every workspace user can edit, stops the run before anything is applied.""" if compiled.space is not None: - _client(client_factory) + client = _client(client_factory) + if compiled.space.parent_path: + _check_folder(client, compiled.space.parent_path, lambda _line: None) def _retire(product_id: str, say: Say, factory: Optional[ClientFactory], dry_run: bool) -> None: @@ -975,7 +1251,7 @@ def _retire(product_id: str, say: Say, factory: Optional[ClientFactory], dry_run try: found = _find(client, marker(product_id)) if found: - found = _owned(client, found, say) + found = _placed(client, found, say)[0] except Exception as exc: # noqa: BLE001 if _status(exc) not in (403, 404): say(f" note: not checked for a Genie space an earlier apply made: {_scrub(exc)}") @@ -1019,37 +1295,49 @@ def _find(client: Any, token: str) -> List[Dict[str, Any]]: query = {"page_size": _PAGE_SIZE, "page_token": token_next} -def _owned(client: Any, found: List[Dict[str, Any]], say: Say) -> List[Dict[str, Any]]: - """The spaces in ``found`` that this identity made: the ones in its own folder, - ``/Users/``, where the Genie API puts a space it creates (measured +def _placed( + client: Any, found: List[Dict[str, Any]], say: Say, folder: str = "" +) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]: + """``found`` by folder: ``(the product's spaces, this identity's own copies)``. + + The product's are the spaces in ``folder`` itself (``parentPath``, compared as + :func:`workspace_folder` writes it, case and all) or, without one, in this identity's own + folder, ``/Users/``, where the Genie API puts a space it creates (measured 2026-11-12; the list leaves ``parent_path`` out, a plain GET has it). A service principal's folder is ``/Users/``, its userName, the folder Databricks Asset - Bundles' default ``root_path`` builds from ``workspace.current_user.userName``. + Bundles' default ``root_path`` builds from ``workspace.current_user.userName``. With + ``folder``, the spaces in this identity's own folder are its copies, from an apply before + ``parentPath`` was set. A space anyone else made carrying the marker is left alone, with a note: adopting it would fill it with the product, share it to the readers and keep its maker's ``CAN_MANAGE`` (inherited from their folder, which a share cannot remove), and that level can read the - readers' conversations. Only this identity and an admin can make a space in its folder. A - failure to read a space's folder other than 403 or 404 is raised, not taken for "someone - else's": that would make a second space for the product. + readers' conversations. Only this identity and an admin can make a space in its folder, and + only whom ``folder``'s permissions let (:func:`_check_folder`) in that one. A failure to read + a space's folder other than 403 or 404 is raised, not taken for "someone else's": that would + make a second space for the product. """ if not found: - return [] + return [], [] home = _home(client) - mine = [] + product: List[Dict[str, Any]] = [] + own: List[Dict[str, Any]] = [] for space in found: space_id = str(space.get("space_id")) parent = _parent(client, space_id) - if _in_folder(parent, home): - mine.append(space) - continue - say( - f" note: left alone: space {space_id} ({space.get('title')!r}) carries " - f"{_marker_of(space)} but is in {parent or 'a folder this identity cannot read'}, not " - "in this identity's own folder, so it is not taken for the product's. Trash it, or " - "take the marker out of its description." - ) - return mine + if folder and workspace_folder(parent) == folder: + product.append(space) + elif _in_folder(parent, home): + (own if folder else product).append(space) + else: + say( + f" note: left alone: space {space_id} ({space.get('title')!r}) carries " + f"{_marker_of(space)} but is in {parent or 'a folder this identity cannot read'}, " + "not in " + (f"{folder} or " if folder else "") + "this identity's own folder, " + "so it is not taken for the product's. Trash it, or take the marker out of its " + "description." + ) + return product, own def _home(client: Any) -> str: @@ -1098,7 +1386,10 @@ def _body(space: Space, serialized: Mapping[str, Any], title: str = "") -> Dict[ def _create(client: Any, space: Space, serialized: Mapping[str, Any]) -> Dict[str, Any]: - return client.api_client.do("POST", SPACES_PATH, body=_body(space, serialized)) or {} + body = _body(space, serialized) + if space.parent_path: + body["parent_path"] = space.parent_path + return client.api_client.do("POST", SPACES_PATH, body=body) or {} def _update( @@ -1117,7 +1408,7 @@ def _update( except Exception as exc: # noqa: BLE001 — a 409 is a taken title or someone else's edit if _status(exc) == 409 and _taken_title(exc): raise GenieError( - f"another Genie space in this identity's folder is already called " + f"another Genie space in {space.folder} is already called " f"{body['title']!r}, and Genie keeps titles unique there.", ( f"Give this space a title of its own with extensions.{EXTENSION_KEY}.title, " diff --git a/tests/iac/_fake_databricks.py b/tests/iac/_fake_databricks.py index 186c0de9..a513764b 100644 --- a/tests/iac/_fake_databricks.py +++ b/tests/iac/_fake_databricks.py @@ -115,6 +115,24 @@ def _err(status: int, code: str, msg: str) -> Tuple[int, Dict[str, Any]]: return status, {"error_code": code, "message": msg} +#: A service principal's application id, the name of its /Users folder. +_APP_ID = re.compile(r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}", re.I) + + +def _bare(path: str) -> str: + """A workspace path as the service reads it back: without the ``/Workspace`` prefix a + create accepts and without a trailing ``/`` (measured 2026-11-12).""" + if path == "/Workspace" or path.startswith("/Workspace/"): + path = path[len("/Workspace") :] + return path.rstrip("/") or "/" + + +def _folder_chain(folder: str) -> List[str]: + """``folder`` and every folder above it, the root first.""" + parts = [p for p in folder.split("/") if p] + return ["/"] + ["/" + "/".join(parts[: i + 1]) for i in range(len(parts))] + + # ---------------------------------------------------------------------- SQL interpreter def _skip_quoted(s: str, i: int) -> int: """Index just past the quoted run (' or `) that starts at ``s[i]``.""" @@ -1055,6 +1073,16 @@ def __init__( # workspace), and the permissions set on each one directly: {(key, name): level}. self.genie_spaces: Dict[str, Dict[str, Any]] = {} self.genie_acl: Dict[str, Dict[Tuple[str, str], str]] = {} + # Workspace folders' own permissions, {folder: {(key, name): level}}, over the ones a + # live workspace starts with (measured 2026-11-12, ``_folder_own_acl``). A folder and a + # space inherit every permission of the folders above them. + self.folder_acl: Dict[str, Dict[Tuple[str, str], str]] = {} + # True: a create or move answers 200 and leaves the space where it was (a service that + # stopped reading parent_path, which is Public Preview) + self.genie_ignores_parent_path = False + # Where a create that names no parent_path puts the space: the deploy user's own folder, + # unless a test that switches identities (me_user_name) moves it to the new one's + self.genie_default_parent: Optional[str] = None # metric views by full name: {"source": table full name, "comment": ...} self.metric_views: Dict[str, Dict[str, Any]] = {} self._space_seq = 0 @@ -1361,6 +1389,7 @@ def _register(self) -> None: a("DELETE", r"/api/2\.0/genie/spaces/([^/]+)", self._genie_trash) a("GET", r"/api/2\.0/permissions/genie/([^/]+)", self._genie_get_acl) a("PUT", r"/api/2\.0/permissions/genie/([^/]+)", self._genie_put_acl) + a("GET", r"/api/2\.0/permissions/directories/([0-9]+)", self._folder_get_acl) # ------------------------------------------------------------------ handlers def _me(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: @@ -1644,10 +1673,14 @@ def _genie_create(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: problem = self._genie_check(b.get("serialized_space")) if problem: return _err(400, "INVALID_PARAMETER_VALUE", problem) + refused = self._genie_parent_refused(b) + if refused: + return refused with self._lock: self._space_count += 1 space_id = f"01f1{self._space_count:028x}" - parent = b.get("parent_path") or f"/Users/{_USER}" + asked = None if self.genie_ignores_parent_path else b.get("parent_path") + parent = _bare(asked or self.genie_default_parent or f"/Users/{_USER}") title = b.get("title") or "" if self._genie_title_taken(title, parent): # The service keeps titles unique in a folder by suffixing the new one. @@ -1683,6 +1716,9 @@ def _genie_patch(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: return _err(404, "RESOURCE_DOES_NOT_EXIST", f"Space {space_id} does not exist.") if b.get("etag") and b["etag"] != space["etag"]: return _err(409, "ABORTED", "The space was modified; the etag does not match.") + refused, b = self._genie_moved(b) + if refused: + return refused if "serialized_space" in b: problem = self._genie_check(b["serialized_space"]) if problem: @@ -1710,40 +1746,112 @@ def _genie_get_acl(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: space_id = m.group(1) if space_id not in self.genie_spaces: return _err(404, "RESOURCE_DOES_NOT_EXIST", f"Space {space_id} does not exist.") - # The creator's and the admins' CAN_MANAGE come from the folders, as on a live - # workspace; only what was set on the space itself is direct. - acl: List[Dict[str, Any]] = [ - { - "user_name": _USER, - "all_permissions": [ - { - "permission_level": "CAN_MANAGE", - "inherited": True, - "inherited_from_object": ["/directories/1"], - } - ], - }, - { - "group_name": "admins", - "all_permissions": [ + # Every level on the space's folder and the folders above it is inherited, as on a live + # workspace (the creator's CAN_MANAGE comes from their folder); only what was set on the + # space itself is direct. + folders = _folder_chain(self.genie_spaces[space_id]["parent_path"]) + return 200, { + "object_id": f"/genie/{space_id}", + "object_type": "genie", + "access_control_list": self._acl_doc(folders, self.genie_acl[space_id]), + } + + # ---- workspace folders: what a Genie space's parent_path must be, and their permissions + + def _folder_own_acl(self, folder: str) -> Dict[Tuple[str, str], str]: + """The levels set on ``folder`` itself: ``folder_acl``'s, else the ones a live workspace + starts with (measured 2026-11-12): the root grants admins CAN_MANAGE, /Shared grants + users CAN_MANAGE, a /Users/ folder grants CAN_MANAGE.""" + if folder in self.folder_acl: + return self.folder_acl[folder] + if folder == "/": + return {("group_name", "admins"): "CAN_MANAGE"} + if folder == "/Shared": + return {("group_name", "users"): "CAN_MANAGE"} + parts = folder.split("/") + if len(parts) == 3 and parts[1] == "Users" and parts[2]: + key = "service_principal_name" if _APP_ID.fullmatch(parts[2]) else "user_name" + return {(key, parts[2]): "CAN_MANAGE"} + return {} + + def _is_folder(self, folder: str) -> bool: + parts = folder.split("/") + home = len(parts) == 3 and parts[1] == "Users" and bool(parts[2]) + return folder in ("/", "/Shared", "/Users") or home or folder in self.workspace_dirs + + def _folder_id(self, folder: str) -> int: + """A folder's object_id: the root's is 0, as on a live workspace.""" + if folder == "/": + return 0 + with self._lock: + return self._object_ids.setdefault(folder, len(self._object_ids) + 1) + + def _acl_doc( + self, inherited_from: Sequence[str], direct: Dict[Tuple[str, str], str] + ) -> List[Dict[str, Any]]: + """``access_control_list`` as the Permissions API answers it: one entry per principal, + each level it holds from the folders ``inherited_from`` (the root renders as + ``/directories/``, as measured) and the ones set directly.""" + held: Dict[Tuple[str, str], List[Dict[str, Any]]] = {} + for folder in inherited_from: + source = f"/directories/{self._folder_id(folder) or ''}" + for key, level in sorted(self._folder_own_acl(folder).items()): + held.setdefault(key, []).append( { - "permission_level": "CAN_MANAGE", + "permission_level": level, "inherited": True, - "inherited_from_object": ["/directories/"], + "inherited_from_object": [source], } - ], - }, - ] - for (key, name), level in sorted(self.genie_acl[space_id].items()): - acl.append( - {key: name, "all_permissions": [{"permission_level": level, "inherited": False}]} - ) + ) + for key, level in sorted(direct.items()): + held.setdefault(key, []).append({"permission_level": level, "inherited": False}) + return [{kind: name, "all_permissions": levels} for (kind, name), levels in held.items()] + + def _folder_get_acl(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: + wanted = int(m.group(1)) + folder = next( + ( + path + for path in ("/", "/Shared", *sorted(self.workspace_dirs), *self._object_ids) + if self._is_folder(path) and self._folder_id(path) == wanted + ), + None, + ) + if folder is None: + return _err(404, "RESOURCE_DOES_NOT_EXIST", f"Directory {wanted} does not exist.") return 200, { - "object_id": f"/genie/{space_id}", - "object_type": "genie", - "access_control_list": acl, + "object_id": f"/directories/{wanted}", + "object_type": "directory", + "access_control_list": self._acl_doc( + _folder_chain(folder)[:-1], self._folder_own_acl(folder) + ), } + def _genie_moved( + self, body: Dict[str, Any] + ) -> Tuple[Optional[Tuple[int, Any]], Dict[str, Any]]: + """A PATCH body with its ``parent_path`` as the service keeps it (no ``/Workspace``, no + trailing ``/``; left out by a service that ignores it), or the refusal of one.""" + refused = self._genie_parent_refused(body) + if refused or "parent_path" not in body: + return refused, body + moved = {**body, "parent_path": _bare(body["parent_path"])} + if self.genie_ignores_parent_path: + moved.pop("parent_path") + return None, moved + + def _genie_parent_refused(self, body: Dict[str, Any]) -> Optional[Tuple[int, Any]]: + """A ``parent_path`` the service refuses, as it refuses it (measured 2026-11-12): a + relative one with 400, a folder that does not exist (case and all) with 404.""" + if "parent_path" not in body: + return None + raw = str(body["parent_path"] or "") + if not raw.startswith("/"): + return _err(400, "BAD_REQUEST", f"requirement failed: {raw} is not absolute.") + if not self._is_folder(_bare(raw)): + return _err(404, "NOT_FOUND", f"Tree node with path {_bare(raw)} does not exist") + return None + def _genie_put_acl(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: space_id = m.group(1) if space_id not in self.genie_spaces: @@ -2085,7 +2193,19 @@ def _ws_status(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: "size": len(self.workspace_files[path]), } if path in self.workspace_dirs: - return 200, {"path": path, "object_type": "DIRECTORY", "object_id": 1} + return 200, { + "path": path, + "object_type": "DIRECTORY", + "object_id": self._folder_id(path), + } + # The folders a live workspace has from the start, with or without /Workspace. + bare = _bare(path) if path.startswith("/") else path + if self._is_folder(bare): + return 200, { + "path": path, + "object_type": "DIRECTORY", + "object_id": self._folder_id(bare), + } return _err(404, "RESOURCE_DOES_NOT_EXIST", f"Path ({path}) doesn't exist.") def _ws_delete(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: diff --git a/tests/iac/test_iac_databricks_genie.py b/tests/iac/test_iac_databricks_genie.py index 35392738..220da99e 100644 --- a/tests/iac/test_iac_databricks_genie.py +++ b/tests/iac/test_iac_databricks_genie.py @@ -624,7 +624,7 @@ def test_a_dry_run_reads_and_writes_nothing(fake): assert run.writes() == [] and fake.genie_spaces == {} assert "would create 'Site 360'" in run.text assert "would share it (CAN_RUN) with" in run.text - assert "dry-run: no Genie space was created, changed or shared." in run.text + assert "dry-run: no Genie space was created, changed, moved, shared or trashed." in run.text Run(fake)(site_contract()) changed = Run(fake)(site_contract(title="Tower Passport"), dry_run=True) @@ -1161,3 +1161,257 @@ def test_a_refused_metric_view_read_is_not_blamed_on_genie(fake): "Check that this identity may read the metric views in Unity Catalog, then re-run " "fluid apply.", ) + + +# ---------------------------------------------------------------------- parentPath: one shared folder + +SALES = "/genie/sales" +CI_SP = "3f2a9c1e-7b4d-4e8a-9f0c-2d6b8e1a5c37" # a CI service principal's application id + + +def in_sales(fake: FakeDatabricks) -> None: + """The folder an admin made for the product, outside /Shared (only admins write the root).""" + fake.workspace_dirs.update({"/genie", SALES}) + + +@pytest.mark.parametrize( + "raw, folder", + [ + (SALES, SALES), + ("/Workspace/genie/sales/", SALES), + (" /genie/sales ", SALES), + ("/WorkspaceX/sales", "/WorkspaceX/sales"), # the prefix counts as a whole segment only + ("/Users/fake-user@example.test/genie", "/Users/fake-user@example.test/genie"), + ], +) +def test_parent_path_is_written_as_the_api_reads_it_back(raw, folder): + assert space_of(site_contract(parentPath=raw)).parent_path == folder + assert space_of(site_contract()).parent_path == "" + + +@pytest.mark.parametrize( + "raw, words", + [ + ("genie/sales", "not absolute"), + ("/", "the workspace root"), + ("/Workspace", "the workspace root"), + ("/genie/../sales", "'..' part"), + ("/genie//sales", "empty"), + ("/genie/./sales", "'.'"), + ("/genie/sales\n", "control character"), + ("", "not a folder path"), + (None, "not a folder path"), + (42, "not a folder path"), + ], +) +def test_a_parent_path_that_names_no_folder_is_refused_by_validate_and_before_tofu(raw, words): + doc = site_contract(parentPath=raw) + result = compiled(doc) + assert [r.kind for r in result.refusals] == ["databricks-genie-invalid"], result.refusals + assert ( + words in str(result.refusals[0]) and "/genie/" in result.refusals[0].remediation[0] + ) + errors, _warnings = dbx.validate_databricks_binding(doc, {"DATABRICKS_WAREHOUSE_ID": "wh-1"}) + assert any("parentPath" in e for e in errors), errors + with pytest.raises(UnsupportedBindingError): + PLUGIN.emit(doc) + + +@pytest.mark.parametrize("raw", ["/Shared", "/Shared/genie", "/Workspace/Shared/genie/sales/"]) +def test_a_parent_path_in_shared_is_refused_offline(raw): + """/Shared grants every workspace user CAN_MANAGE, inherited by everything in it + (measured): they could read every reader's conversations.""" + result = compiled(site_contract(parentPath=raw)) + assert [r.kind for r in result.refusals] == ["databricks-genie-folder"] + message = str(result.refusals[0]) + assert "every workspace user holds CAN_MANAGE" in message + assert "read every reader's conversations" in message + assert "outside /Shared" in result.refusals[0].remediation[0] + assert compiled(site_contract(parentPath="/SharedData/genie")).refusals == [] + + +def test_a_misspelt_parent_path_key_is_named(): + result = compiled(site_contract(parent_path=SALES)) + assert "did you mean 'parentPath'?" in str(result.refusals[0]) + + +def test_a_space_is_made_in_the_parent_path_and_found_there_again(fake): + in_sales(fake) + first = Run(fake)(site_contract(parentPath="/Workspace/genie/sales/")) + space = only_space(fake) + assert space["parent_path"] == SALES + assert "created 'Site 360' in /genie/sales: 1 table(s)" in first.text + assert Run(fake)(site_contract(parentPath=SALES)).writes() == [] + + +def test_another_identity_finds_the_same_space_in_the_parent_path(fake): + """The point of parentPath: a laptop and CI keep one space, not one each.""" + in_sales(fake) + Run(fake)(site_contract(parentPath=SALES)) + fake.me_user_name = CI_SP + second = Run(fake)(site_contract(parentPath=SALES)) + assert len(fake.genie_spaces) == 1 + assert second.writes() == [] and "'Site 360': no change" in second.text + + +def test_without_parent_path_each_identity_keeps_its_own_space(fake): + """Why parentPath exists: CI does not adopt the laptop's space in the laptop's folder.""" + Run(fake)(site_contract()) + laptop = only_space(fake)["space_id"] + fake.me_user_name = CI_SP + fake.genie_default_parent = f"/Users/{CI_SP}" + run = Run(fake)(site_contract()) + assert f"left alone: space {laptop}" in run.text + assert len(fake.genie_spaces) == 2 + + +def test_a_space_in_this_identitys_own_folder_is_moved_into_the_parent_path(fake): + in_sales(fake) + Run(fake)(site_contract()) + space_id = only_space(fake)["space_id"] + dry = Run(fake)(site_contract(parentPath=SALES), dry_run=True) + assert dry.writes() == [] and f"would move space {space_id}" in dry.text + run = Run(fake)(site_contract(parentPath=SALES)) + assert set(fake.genie_spaces) == {space_id} and only_space(fake)["parent_path"] == SALES + assert run.writes()[0] == ("PATCH", f"/api/2.0/genie/spaces/{space_id}") + assert f"moved space {space_id} from this identity's own folder into /genie/sales" in run.text + assert Run(fake)(site_contract(parentPath=SALES)).writes() == [] + + +def test_a_copy_in_this_identitys_own_folder_is_trashed_once_the_space_is_in_the_folder(fake): + in_sales(fake) + Run(fake)(site_contract(parentPath=SALES)) + shared = only_space(fake)["space_id"] + fake.me_user_name = CI_SP + old = fake.add_genie_space( + "Site 360", f"CI's own, from before {MARKER}", parent_path=f"/Users/{CI_SP}" + ) + dry = Run(fake)(site_contract(parentPath=SALES), dry_run=True) + assert dry.writes() == [] and f"would trash 'Site 360' ({old})" in dry.text + run = Run(fake)(site_contract(parentPath=SALES)) + assert set(fake.genie_spaces) == {shared} + assert ("DELETE", f"/api/2.0/genie/spaces/{old}") in run.writes() + assert "a copy in this identity's own folder: the product's space is the one in" in run.text + + +def test_two_copies_in_this_identitys_own_folder_are_not_guessed_between(fake): + in_sales(fake) + a = fake.add_genie_space("Site 360", f"one {MARKER}") + b = fake.add_genie_space("Site 360 2026-11-12 20:35:27", f"two {MARKER}") + run = Run(fake) + with pytest.raises(genie.GenieError) as exc: + run(site_contract(parentPath=SALES)) + assert a in str(exc.value) and b in str(exc.value) and run.writes() == [] + + +@pytest.mark.parametrize( + "where", + [ + "/Users/mallory@towers.io", + "/genie", # the folder above + "/genie/sales/drafts", # a folder inside: its permissions can be wider + "/Genie/sales", # workspace paths are case-sensitive + ], +) +def test_a_marker_space_beside_the_parent_path_is_left_alone(fake, where): + in_sales(fake) + fake.workspace_dirs.update({"/genie/sales/drafts", "/Genie", "/Genie/sales"}) + decoy = fake.add_genie_space("Site 360", f"planted {MARKER}", parent_path=where) + before = copy.deepcopy(fake.genie_spaces[decoy]) + run = Run(fake)(site_contract(parentPath=SALES)) + assert fake.genie_spaces[decoy] == before and fake.genie_acl[decoy] == {} + assert f"left alone: space {decoy}" in run.text + assert f"not in {SALES} or this identity's own folder" in run.text + (ours,) = set(fake.genie_spaces) - {decoy} + assert fake.genie_spaces[ours]["parent_path"] == SALES + + +def test_a_missing_parent_folder_stops_the_apply_before_tofu(fake): + doc = site_contract(parentPath=SALES) + with pytest.raises(genie.GenieError) as exc: + PLUGIN.pre_apply(doc, say=lambda _line: None, client_factory=fake.client) + assert "the workspace has no such folder" in str(exc.value) + assert "A workspace admin creates /genie/sales" in exc.value.remediation[0] + for dry_run in (True, False): + run = Run(fake) + with pytest.raises(genie.GenieError): + run(doc, dry_run=dry_run) + assert run.writes() == [] + assert fake.genie_spaces == {} + + +@pytest.mark.parametrize( + "folder, level, harm", + [ + ("/genie", "CAN_MANAGE", "read every reader's conversations"), # inherited + (SALES, "CAN_EDIT", "edit the space's instructions"), + ], +) +def test_a_parent_folder_every_user_can_edit_is_refused(fake, folder, level, harm): + in_sales(fake) + fake.folder_acl[folder] = {("group_name", "users"): level} + doc = site_contract(parentPath=SALES) + with pytest.raises(genie.GenieError) as before_tofu: + PLUGIN.pre_apply(doc, say=lambda _line: None, client_factory=fake.client) + run = Run(fake) + with pytest.raises(genie.GenieError) as exc: + run(doc) + for error in (before_tofu.value, exc.value): + assert f"every workspace user holds {level} on the folder {SALES}" in str(error) + assert harm in str(error) + assert "away from users" in error.remediation[0] + assert run.writes() == [] and fake.genie_spaces == {} + + +def test_a_parent_folder_every_user_can_open_is_said_not_refused(fake): + in_sales(fake) + fake.folder_acl[SALES] = {("group_name", "users"): "CAN_RUN"} + run = Run(fake)(site_contract(parentPath=SALES)) + assert "note: every workspace user holds CAN_RUN on /genie/sales" in run.text + assert only_space(fake)["parent_path"] == SALES + + +def test_folder_permissions_this_identity_cannot_read_stop_the_apply(fake): + in_sales(fake) + fake.fail_route("GET", "/api/2.0/permissions/directories/", 403, "no CAN_MANAGE") + run = Run(fake) + with pytest.raises(genie.GenieError) as exc: + run(site_contract(parentPath=SALES)) + assert "could not read the permissions on the folder /genie/sales" in str(exc.value) + assert run.writes() == [] + + +def test_a_parent_path_that_is_a_file_is_refused(fake): + fake._put_file("/genie/sales", b"not a folder") + with pytest.raises(genie.GenieError) as exc: + Run(fake)(site_contract(parentPath=SALES)) + assert "which is a file, and a Genie space goes in a folder" in str(exc.value) + + +def test_a_new_space_that_does_not_land_in_the_parent_path_is_trashed(fake): + in_sales(fake) + fake.genie_ignores_parent_path = True + with pytest.raises(genie.GenieError) as exc: + Run(fake)(site_contract(parentPath=SALES)) + assert "not in /genie/sales, where the next apply looks for it" in str(exc.value) + assert fake.genie_spaces == {} + + +def test_a_move_the_service_does_not_make_stops_the_apply(fake): + in_sales(fake) + Run(fake)(site_contract()) + fake.genie_ignores_parent_path = True + with pytest.raises(genie.GenieError) as exc: + Run(fake)(site_contract(parentPath=SALES)) + assert "accepted the move" in str(exc.value) and "by hand" in exc.value.remediation[0] + + +def test_dropping_the_block_leaves_a_space_in_the_parent_path_with_a_note(fake): + """Without the block the apply no longer knows which folder it trusts, so it does not trash + a space there; the note names it for the person removing the block.""" + in_sales(fake) + Run(fake)(site_contract(parentPath=SALES)) + space_id = only_space(fake)["space_id"] + run = Run(fake)(without_block(site_contract())) + assert set(fake.genie_spaces) == {space_id} and run.writes() == [] + assert f"left alone: space {space_id}" in run.text and SALES in run.text diff --git a/tests/iac/test_iac_databricks_genie_fake_e2e.py b/tests/iac/test_iac_databricks_genie_fake_e2e.py index 8e60457c..96f65b45 100644 --- a/tests/iac/test_iac_databricks_genie_fake_e2e.py +++ b/tests/iac/test_iac_databricks_genie_fake_e2e.py @@ -137,3 +137,27 @@ def test_an_invalid_block_is_refused_before_tofu_runs(workspace, fake, capfd): rc, out = apply_contract(capfd, path) assert rc != 0 and "databricks-genie-invalid" in out assert fake.tables == {} and fake.genie_spaces == {} + + +def test_a_shared_folder_keeps_one_space_for_two_identities(workspace, fake, capfd): + """``parentPath``: a laptop and CI deploy one product and keep one space.""" + path = write_contract(workspace, genie_contract(parentPath="/genie/orders")) + # The folder a workspace admin makes first: without it the apply stops before tofu. + rc, out = apply_contract(capfd, path) + assert rc != 0 and "the workspace has no such folder" in out, out + assert fake.tables == {} and fake.genie_spaces == {} + + fake.workspace_dirs.update({"/genie", "/genie/orders"}) + rc, out = apply_contract(capfd, path) + assert rc == 0, out + (space,) = fake.genie_spaces.values() + assert space["parent_path"] == "/genie/orders" + assert "created 'Databricks test orders' in /genie/orders" in out + + # CI applies the same contract as its own service principal: the same space, unchanged. + fake.me_user_name = "3f2a9c1e-7b4d-4e8a-9f0c-2d6b8e1a5c37" + rc, out = apply_contract(capfd, path) + assert rc == 0, out + assert "'Databricks test orders': no change" in out + assert len(fake.genie_spaces) == 1 + assert fake.unknown == [] From 7ca989232df316b949aa248f0f741cd0dab4b465 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 23:31:33 +0100 Subject: [PATCH 2/7] feat(databricks): fluid plan and fluid diff list the Genie space; generate iac notes it fluid plan appends one databricks_genie_space action (title, folder, tables, readers) through a new optional plugin hook, planned_output_ports, beside converge_output_ports. The Databricks native planner, which fluid diff reads for the contract's resources, lists the same action. Nothing compares the action with an apply, so it never reads as drift: - diff's verdict for Databricks reads the live tables and the tofu state; - emit ignores plan actions; - the shadow comparison counts no kind for its op. A managed streaming plan gets none, since its apply checks actions one for one. fluid generate iac prints a note, from a new optional plugin hook standalone_module_notices, that the module holds no Genie space, so a plain tofu apply makes none. The pattern follows AWS CDK's synth-time Annotations.addInfo. HONESTLY_TESTED gains the unit, stand-in and live rows for both commits. --- .secrets.baseline | 4 +- AGENTS.md | 3 +- CHANGELOG.md | 5 ++ HONESTLY_TESTED.md | 5 +- fluid_build/cli/generate_iac.py | 21 ++++- fluid_build/cli/plan.py | 58 ++++++++++++ fluid_build/iac/providers/databricks.py | 30 ++++++- fluid_build/iac/providers/databricks_genie.py | 44 +++++++++ tests/iac/test_iac_databricks_genie.py | 89 +++++++++++++++++++ 9 files changed, 252 insertions(+), 7 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 671caa03..3e5f9faf 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -157,7 +157,7 @@ "filename": "AGENTS.md", "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", "is_verified": false, - "line_number": 641 + "line_number": 652 } ], "Jenkinsfile": [ @@ -2317,5 +2317,5 @@ } ] }, - "generated_at": "2026-11-12T20:45:31Z" + "generated_at": "2026-11-12T22:33:39Z" } diff --git a/AGENTS.md b/AGENTS.md index 161c4f87..11935c68 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -375,7 +375,8 @@ in the identity's own folder is moved in, keeping its conversations. A space inh permissions, so the folder is the trust boundary. `/Shared` is refused offline, because every workspace user holds `CAN_MANAGE` there. The apply also stops, before `tofu`, on a folder that does not exist or one where the `users` group can edit or manage. Removing the block leaves a -space in `parentPath` with a note naming it. +space in `parentPath` with a note naming it. `fluid plan` lists the space as a +`databricks_genie_space` action, and `fluid generate iac` notes that its module holds none. Code: `fluid_build/iac/providers/databricks_genie.py`. --- diff --git a/CHANGELOG.md b/CHANGELOG.md index fe222fa5..4815ed66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 whole block leaves a space in `parentPath` in place, with a note naming it: without the block, the apply no longer knows which folder to trust. +- **`fluid plan` lists the Genie space** as a `databricks_genie_space` action with its title, + folder, tables and readers. `fluid diff` counts it among the contract's resources and never + reports it as drift, since nothing compares it with an apply. **`fluid generate iac` adds a + note** that the module it writes holds no Genie space, so a plain `tofu apply` makes none. + - **Databricks: Unity Catalog primary and foreign keys from the contract.** A column tagged `primary-key` (all of them, in schema order, for a composite key; each must be `required: true`) becomes the table's PRIMARY KEY, and one tagged `foreign-key` with diff --git a/HONESTLY_TESTED.md b/HONESTLY_TESTED.md index 55ce77ba..08bb9d9f 100644 --- a/HONESTLY_TESTED.md +++ b/HONESTLY_TESTED.md @@ -94,9 +94,10 @@ Markers declared in `pyproject.toml`. | Manual live, Free Edition (founder-approved) | the real `fluid` CLI, no test file: run 2026-11-01 from main dfd6f145, databricks-sdk 0.158.0, every `fluid_iactest_*` object swept after (1 schema, 1 table, 1 folder; 0 left) | ✅ validate, `generate iac --validate`, plan, dry-run and apply `+9`; a second plan is EMPTY against the real Jobs API; Databricks accepts the Quartz schedule, serverless environment 2 with `dbt-databricks>=1.8.0,<2.0.0` (dbt 1.12.6, adapter 1.13.0), the WORKSPACE `project_directory` and the commands as emitted; one `run-now` ends SUCCESS and dbt builds the table; `fluid verify` with the real SDK (columns, types, one job); `fluid diff`'s state check (its live check has no Databricks inspector); a model change `~1`; the build made local removes job, files and folders `-5` without `--allow-data-loss` (the empty `.fluid//` folders above `files/` stay: only `files/` and below are managed). It found the USE CATALOG check reading a group-conveyed privilege as none. After the fix, `fluid verify` on a fresh one-table product (`+4`) passes every dimension, catalog access included (1 schema and 1 table swept, 0 left). | A GIT source, profile or OAuth auth, a run-as service principal, table adoption and a larger project, live; the region residency check (the metastore precondition and `fluid verify`'s residency dimension), which was added after this run and not run live; dbt's CREATE OR REPLACE drops a contract's NOT NULL, so verify flags a `required` column a dbt model does not constrain itself. | | Manual live, Free Edition (founder-approved), masks and row filters | the real `fluid` CLI, no test file: run 2026-11-03 at the tip of #785, a real workspace with an AWS us-west-2 metastore; every object swept after (0 left) | ✅ 10 steps passed. Functions are created and read back (owner is the user's email; `routine_definition` is the body without `RETURN`, backticks kept; `full_data_type` is STRING or BOOLEAN; a zero-parameter function's FunctionInfo has no `input_params`). The row filter and masks are bound and enforced on the restricted user's own SELECT: only the consented rows, `msisdn` as `XXXXX4567`, a 3-character value hashed, the denied column NULL. `SET ROW FILTER ... ON ()` is accepted and a `SET` replaces a live filter. Tags are present, `fluid verify` passes every dimension, a second apply is `+0 ~0 -0`, the residency precondition fails the plan and names both regions, `generate iac` refuses, and the sweep left 0. | Still open, as this run showed: superseded functions were left behind (orphans; the follow-up that drops them after a successful apply is not run live); `tofu destroy` of the module fails at a schema that still holds functions, so a teardown must drop them first; and the residency failure text was forge-cli's 800-character tail of the tofu JSON, which cut off the sentence naming the live region (the one-line explanation is not run live). A metastore in another cloud or region, a service principal as the run identity, and a group-conveyed grant on the live workspace are still not run. | | Manual live, Free Edition (founder-approved), 2026-11-04 | the real `fluid` CLI, no test file: #801 + #831 at a7169202, run by the demo session, every object swept back to the baseline after | ✅ Unity Catalog metadata read back live: the table and column comments, the product tags (`fluid_product_id` and the rest), the owner-team tag, the `fluid_sensitivity` column tags and `system.certification_status = certified` (accepted for an admin personal access token). The bronze landing: 5 rows loaded, the email column hashed, the staged files deleted from the volume; a second run overwrote the table (the Delta history shows `numRemovedFiles=1`). The column masks, the row filter and a scoped grant were enforced after the `INSERT OVERWRITE`. `lifecycle.retention: P30D` with `expire: true` was accepted, `fluid verify` passed, and predictive optimization was on. A second apply planned `+0 ~0 -0`. A changed row filter bound the new `fluid_rf_` function and dropped the superseded one. Findings fixed in the follow-up PR: the landing left the empty `///` directories and the local masked copy under `.fluid/staging//`, and a dry run did not list the superseded function drops. | Not tested live: `system.certification_status` set by a non-admin identity holding only ASSIGN on the governed tag (only an admin token was used). Open: `facets.databricks_load.rows` counts the rows loaded, which a row-filtered `SELECT COUNT(*)` by a restricted reader does not match (by design of the filter, not reconciled in the run record). | -| Stage 1 unit: Databricks Genie space (`extensions.databricksGenie`) | `tests/iac/test_iac_databricks_genie.py` (the compile, the plugin's `converge_output_ports` against the stand-in's Genie and Permissions APIs, the engine hook) | ✅ 90. What the space holds (sample questions split on `\|`, 32-hex ids sorted, column descriptions with allowed values, range, example and synonyms, one instruction with the metrics' SQL, `agentPolicy` and the access statement), prompt matching never on, a sensitive column's example left out, the refusals (`fluid validate` and before `tofu`), create + share then no change, an update with the `etag`, a 409 not overwritten, a hand edit set back, removal trashes, a stranger's space untouched, a space planted with the marker in another folder never adopted, shared or trashed, a marker quoted inside a description no match, two markers refused, an apply straight after a create finding the space (the list lags), a title another space has kept as the service suffixed it and a rename onto one said as such, a transient failure to read a space's folder failing the apply instead of making a second space, a new space outside the folder the next apply looks in trashed and said, a policy that allows `analysis` but not `qa` accepted and one that denies `analysis` refused, its lines printed literally (Rich read the marker and a `[/x]` title as markup), a principal granted `manage` (`ALL_PRIVILEGES`) counted as a reader, metric views counted toward the 50-entry cap, remediation by HTTP status, a missing SDK stopping the apply before `tofu`, the native plan refusing what `emit` refuses, one snippet per metric name, a principal the service keeps in another case not shared again, a dry run writes nothing, a metric view used once it exists and sent where the service keeps it, readers' `CAN_RUN` with editors kept, the real SDK's error classes read as 403/404/409, no credential in a message, the stand-in refusing what the service refuses, and measure SQL identical to the governed MCP query path's | Genie's answers (only the live row below asks one) | -| Stage 1 real tofu, stand-in workspace: Genie space | `tests/iac/test_iac_databricks_genie_fake_e2e.py` | ✅ 3. The real `fluid` CLI and `tofu`: validate, dry-run, apply (the space after the tables, shared with the readers), a second apply that changes nothing, the block removed (trashed); a Genie failure fails the apply after the tables with its fix and the next apply converges; an invalid block refused before `tofu` | The stand-in is modelled on the measured API, not the service itself | +| Stage 1 unit: Databricks Genie space (`extensions.databricksGenie`) | `tests/iac/test_iac_databricks_genie.py` (the compile, the plugin's `converge_output_ports` against the stand-in's Genie and Permissions APIs, the engine hook) | ✅ 131. What the space holds (sample questions split on `\|`, 32-hex ids sorted, column descriptions with allowed values, range, example and synonyms, one instruction with the metrics' SQL, `agentPolicy` and the access statement), prompt matching never on, a sensitive column's example left out, the refusals (`fluid validate` and before `tofu`), create + share then no change, an update with the `etag`, a 409 not overwritten, a hand edit set back, removal trashes, a stranger's space untouched, a space planted with the marker in another folder never adopted, shared or trashed, a marker quoted inside a description no match, two markers refused, an apply straight after a create finding the space (the list lags), a title another space has kept as the service suffixed it and a rename onto one said as such, a transient failure to read a space's folder failing the apply instead of making a second space, a new space outside the folder the next apply looks in trashed and said, a policy that allows `analysis` but not `qa` accepted and one that denies `analysis` refused, its lines printed literally (Rich read the marker and a `[/x]` title as markup), a principal granted `manage` (`ALL_PRIVILEGES`) counted as a reader, metric views counted toward the 50-entry cap, remediation by HTTP status, a missing SDK stopping the apply before `tofu`, the native plan refusing what `emit` refuses, one snippet per metric name, a principal the service keeps in another case not shared again, a dry run writes nothing, a metric view used once it exists and sent where the service keeps it, readers' `CAN_RUN` with editors kept, the real SDK's error classes read as 403/404/409, no credential in a message, the stand-in refusing what the service refuses, and measure SQL identical to the governed MCP query path's. `parentPath` (one shared folder): written as the API reads it back (no `/Workspace` prefix, no trailing `/`, case kept), a relative, root, `.`/`..` or non-string path and anything under `/Shared` refused offline, the space made in the folder and found there by a second identity, each identity keeping its own without it, a space in the identity's own folder moved in (dry run says so) and a copy left there trashed, two copies not guessed between, a marker space in the folder above, a folder inside, another case or someone's home left alone, a missing folder, a file, a folder `users` can edit or manage (directly or inherited) and unreadable folder permissions stopping the apply before `tofu`, `users` with `CAN_RUN` said, a create or move the service does not make stopping the apply, the block removed leaving the folder's space with a note; the `databricks_genie_space` plan action (`fluid plan`, `fluid diff`'s added list with no drift under `--exit-on-drift`, no shadow-parity kind, the module unchanged) and `fluid generate iac`'s note | Genie's answers (only the live row below asks one) | +| Stage 1 real tofu, stand-in workspace: Genie space | `tests/iac/test_iac_databricks_genie_fake_e2e.py` | ✅ 4. The real `fluid` CLI and `tofu`: validate, dry-run, apply (the space after the tables, shared with the readers), a second apply that changes nothing, the block removed (trashed); a Genie failure fails the apply after the tables with its fix and the next apply converges; an invalid block refused before `tofu`; `parentPath`: a missing folder stops the apply before `tofu` writes a table, then the space made in the folder and a second identity's apply changing nothing | The stand-in is modelled on the measured API, not the service itself | | Manual live, Free Edition, Genie space, 2026-11-12 | the real `fluid` CLI, no test file: branch `feat/databricks-genie-space`, databricks-sdk 0.162.0, a copy of industry-models tower-management `dp019_site_360` in the scratch catalog `fluid_iactest_genie` (no dbt build, no MENA sovereignty block: the workspace is in us-west-2), 6 synthetic rows | ✅ dry-run (would create, would share), apply `+73` then the space created and shared `CAN_RUN` with two persona service principals, a second apply `+0 ~0 -0` and "no change", the question "Which on-air sites in Amman are colocation-ready with hybrid power and no standing risk?" answered through the Genie Conversation API with the 2 expected sites (its SQL took the latest `snapshot_date` and the inverse of the `at_risk_site_count` filter from the instruction), a metric view created by hand picked up ("updated: metric views", then "no change"), the block removed (trashed). Two defects found and fixed in the run: the metric-view read failed on a schema not created yet, and the service keeps a metric view among the tables. A re-check after the security review (the plugin's own hook over `samples.nyctaxi.trips`, nothing created in Unity Catalog) found the list shows a new space only ~9 s later (a run straight after made a second space) and that a taken title is suffixed on create and 409 on PATCH; fixed, then create, an immediate second run ("no change", same space), and the block removed (trashed, via the folder check) passed live. Swept: the space trashed, `DROP CATALOG fluid_iactest_genie CASCADE`, 0 left | A reader without `SELECT` asking (no persona credential here; the access model is Genie's documented one); a service principal as the deploying identity (its folder is `/Users/`, the one Databricks Asset Bundles build from `current_user.userName`; a create that lands anywhere else is trashed and stops the apply); a space over several products | +| Manual live, Free Edition, Genie `parentPath`, 2026-11-12 | the real `fluid` CLI, no test file: branch `feat/databricks-genie-shared-folder` on main 97811add, databricks-sdk 0.162.0, a one-table product in the scratch catalog `fluid_iactest_genie_pp`, its folder `/fluid_iactest_genie_pp` at the workspace root | ✅ A probe measured the API first. A create accepts `/Users/`, `/Workspace/Users/` (accepted this time; an earlier probe that day had a 400), `/Shared/x`, `/Workspace/Shared/x`, a trailing `/` and a root-level folder, and a GET returns the bare form. A missing folder is 404, a relative path 400, and `/shared/x` 404 (paths are case-sensitive). `/Shared` grants `users` `CAN_MANAGE` directly, and every folder and space under it inherits that; the root grants only admins. A space's permissions all come from its folder, and `PATCH {parent_path}` moves it. Then through the CLI: validate refuses `/Workspace/Shared/...`; an apply with the folder missing stops after `tofu plan`, before `tofu apply` (no schema made); a space made without `parentPath` in the user's folder, a dry run says it would move, the apply moves it (same id), and a second apply changes nothing; `users` with `CAN_MANAGE` on the folder is refused before `tofu apply`, and `users` with `CAN_RUN` is a note; `fluid plan` lists `genie_space_`, `generate iac` prints the note, and `diff --exit-on-drift` exits 0 with the space among `added`; with the block removed, the folder's space is left with a note. Swept: the space trashed, the folder deleted, the catalog dropped, 0 left | A second identity, live (no service-principal secret here; the stand-in e2e covers it); a folder a non-admin made inside someone's home | | Stage 2 emulator (nightly, non-gating) | `tests/iac/test_iac_databricks_minilake.py` (job `nightly-databricks-minilake`) | ⚠️ 1 passed, 4 strict xfails (manual dispatch, run 42267640334, 2026-11-06, minilake v1.9.0). Schema, table and grant creates work, the replace gate holds, adding a column plans no replace, and a grant revoke is classified not gated. The four tests about those resources apply the module without its tag assignments (the `untagged` fixture). An xfail is satisfied only by its own gap's failure (`MinilakeGap`), and a pass fails the run, so an image that closes a gap turns the job red until the mark comes off. Earlier: 1/4 from the first run (2026-10-30) to the 2026-11-04 nightly, then 0/4 after #831, every apply stopping at the first tag. | A second plan is not empty and no in-place table update applies: minilake drops a CREATE TABLE's `TBLPROPERTIES` and `COMMENT`, and DuckDB cannot parse the `ALTER TABLE ... SET TBLPROPERTIES` the provider sends on every update of a table that sets properties. No tag is applied: minilake has no entity-tag-assignments API (it answers 501). v1.9.3 (2026-11-05) has both gaps, so the pin stays; an upstream issue for each is drafted, not filed. Jobs and workspace files are not in this flow yet. | | Stage 3 live | — | ❌ not automated: manual maintainer runs only, never CI. Two have run: the manual rows above (2026-11-01 and 2026-11-03). | Everything the rows above leave to the live tier. | diff --git a/fluid_build/cli/generate_iac.py b/fluid_build/cli/generate_iac.py index 10afbb5f..57f19312 100644 --- a/fluid_build/cli/generate_iac.py +++ b/fluid_build/cli/generate_iac.py @@ -15,9 +15,10 @@ import argparse import logging import os +from typing import Any, List from fluid_build._errors import FluidUserError -from fluid_build.cli.console import cprint +from fluid_build.cli.console import cprint, cprint_literal from fluid_build.iac import ( IAC_PLUGINS, assemble_tofu_document, @@ -127,6 +128,7 @@ def run(args, logger: logging.Logger) -> int: actions = native_actions(contract, logger, offline=True) resources = plugin.emit(contract, actions) data = plugin.emit_data(contract, actions) + notices = _module_notices(plugin, contract) count = sum(len(items) for items in resources.values()) provider_cfg = provider_config(plugin, contract) document = assemble_tofu_document( @@ -200,12 +202,29 @@ def run(args, logger: logging.Logger) -> int: cprint("\nReview and apply with OpenTofu:") cprint(f" tofu -chdir={out_dir} init") cprint(f" tofu -chdir={out_dir} plan") + _say_notices(notices, logger) if getattr(args, "shadow", False): _print_shadow_report(contract, plugin, logger) return 0 +def _module_notices(plugin: Any, contract: Any) -> List[str]: + """What the module leaves out that `fluid apply` does make (a Databricks Genie space, + through its REST API), from the plugin's optional ``standalone_module_notices``: said with + the module, as AWS CDK's synth prints a construct's ``Annotations.addInfo``, so a reviewer's + plain `tofu apply` holds no surprise.""" + notes = getattr(plugin, "standalone_module_notices", None) + return [str(note) for note in notes(contract)] if callable(notes) else [] + + +def _say_notices(notices: List[str], logger: logging.Logger) -> None: + for notice in notices: + info(logger, "generate_iac_notice", notice=notice) + # Literally: a plugin's note may quote contract text, which Rich would read as markup. + cprint_literal(f"\nNote: {notice}") + + def _refuse_an_unset_aws_account(rendered: str) -> None: """Refuse a module that would carry :data:`UNSET_AWS_ACCOUNT`. diff --git a/fluid_build/cli/plan.py b/fluid_build/cli/plan.py index 964a46cd..822b7905 100644 --- a/fluid_build/cli/plan.py +++ b/fluid_build/cli/plan.py @@ -352,6 +352,10 @@ def run(args, logger: logging.Logger) -> int: plan["actions"] = list(existing) + schedule_actions plan["total_actions"] = len(plan["actions"]) + # What the apply makes outside OpenTofu (a Databricks Genie space), before the digests. + if not streaming: + _append_output_port_actions(plan, contract, logger) + # --- Packaging truthfulness (RFC-packaging-modes.md file 8) --------- # plan.json is the digest-bound artifact the human reviews, and # untouched it would list container-*creation* actions for containers @@ -858,6 +862,60 @@ def _validate_plan_actions(plan: Dict[str, Any], logger: logging.Logger) -> None cprint(f"\n✓ All {len(typed)} actions pass schema validation") +def _append_output_port_actions( + plan: Dict[str, Any], contract: Dict[str, Any], logger: logging.Logger +) -> None: + """Add to ``plan`` the actions an IaC plugin names for what ``fluid apply`` makes after + ``tofu apply`` (its ``planned_output_ports`` hook, beside ``converge_output_ports``): the + Databricks plugin's Genie space. One per platform the contract's exposes bind to, from the + in-tree plugins only (``IAC_PLUGINS``: nothing is loaded from an entry point to plan). They + come last and depend on nothing in the graph, since they run after the whole tofu apply. + The OpenTofu engine applies from the contract, never from plan actions, so they change what + is reviewed, not what is applied; ``planDigest`` covers them. Never on a managed streaming + plan, whose apply runs no OpenTofu and checks its actions against its artifacts one for one. + A hook that fails leaves its actions out with a warning: ``fluid validate`` and ``fluid + apply`` refuse the contract with the reason, and the plan of the rest stays readable.""" + from fluid_build.iac.provider_match import canonical_cloud + + platforms = sorted( + { + canonical_cloud((expose.get("binding") or {}).get("platform")) + for expose in contract.get("exposes") or [] + if isinstance(expose, dict) and isinstance(expose.get("binding"), dict) + } + - {""} + ) + if not platforms: + return + # Function-local: ``iac`` pulls every provider plugin, off the ``fluid --help`` cold path. + from fluid_build.iac import IAC_PLUGINS + + added: List[Dict[str, Any]] = [] + for name in platforms: + hook = getattr(IAC_PLUGINS.get(name), "planned_output_ports", None) + if not callable(hook): + continue + try: + added.extend(dict(action) for action in hook(contract)) + except Exception as exc: # noqa: BLE001 - the plan of everything else still stands + warn( + logger, + "plan_output_ports_skipped", + provider=name, + error=f"{type(exc).__name__}: {exc}", + ) + if not added: + return + actions = list(plan.get("actions") or []) + for action in added: + actions.append({"step": len(actions) + 1, "depends_on": [], **action}) + plan["actions"] = actions + plan["total_actions"] = len(actions) + graph = plan.get("dependency_graph") + if isinstance(graph, dict) and isinstance(graph.get("nodes"), list): + graph["nodes"] = graph["nodes"] + [a.get("action_id") for a in added] + + def _plan_schedule_actions( contract: Dict[str, Any], args, logger: logging.Logger ) -> List[Dict[str, Any]]: diff --git a/fluid_build/iac/providers/databricks.py b/fluid_build/iac/providers/databricks.py index 2867ba78..9f6c9665 100644 --- a/fluid_build/iac/providers/databricks.py +++ b/fluid_build/iac/providers/databricks.py @@ -112,7 +112,8 @@ Genie API after the masks and row filters are in place, and finds it again by a marker in its description (``databricks_genie.py``): in the deploying identity's own folder, or in the workspace folder ``extensions.databricksGenie.parentPath`` names, which a product deployed - from more than one identity shares. + from more than one identity shares. ``fluid plan`` lists the space as a + ``databricks_genie_space`` action, and ``fluid generate iac`` notes that its module holds none. * ``binding.location.region`` is the region the workspace is in, declared so a contract's ``sovereignty`` policy can check it (``policy/sovereignty.py`` @@ -2354,6 +2355,28 @@ def standalone_module_refusal( "Or remove the column restrictions and row filters from the contract.", ) + def planned_output_ports(self, contract: Mapping[str, Any]) -> List[Dict[str, Any]]: + """``fluid plan``'s actions for what :meth:`converge_output_ports` makes after ``tofu + apply``: the Genie space ``extensions.databricksGenie`` asks for + (``databricks_genie.space_plan_action``). A contract the emitter refuses lists none here; + ``fluid validate`` and ``fluid apply`` say why.""" + from .databricks_genie import space_plan_action + + compiled = _compile(contract, os.environ) + if compiled.refusals: + return [] + space = _genie(contract, compiled, os.environ).space + return [space_plan_action(space)] if space is not None else [] + + def standalone_module_notices(self, contract: Mapping[str, Any]) -> List[str]: + """What a module ``fluid generate iac`` writes leaves out that ``fluid apply`` makes: + the Genie space ``extensions.databricksGenie`` asks for (Genie API, after ``tofu + apply``). Printed under the module's path; nothing is refused for it.""" + from .databricks_genie import module_notice + + notice = module_notice(contract) + return [notice] if notice else [] + @staticmethod def _enforcement_targets(contract: Mapping[str, Any]) -> List[Target]: compiled = _compile(contract, os.environ) @@ -2761,6 +2784,11 @@ def planned_objects(contract: Mapping[str, Any]) -> List[Dict[str, Any]]: } ) actions.extend(_job_actions(compiled.jobs)) + if genie.space is not None: + from .databricks_genie import space_plan_action + + # Not in the module: listed so the plan shows what the apply does after tofu apply. + actions.append(space_plan_action(genie.space)) unique: Dict[Tuple[str, str], Dict[str, Any]] = {} for action in actions: unique.setdefault((action["resource_type"], action["resource_id"]), action) diff --git a/fluid_build/iac/providers/databricks_genie.py b/fluid_build/iac/providers/databricks_genie.py index 34f77c02..4baa50de 100644 --- a/fluid_build/iac/providers/databricks_genie.py +++ b/fluid_build/iac/providers/databricks_genie.py @@ -462,6 +462,50 @@ def _policy_refusals(table: Any) -> List[UnsupportedBindingError]: return out +#: The plan op of a Genie space: its words name no kind the engines' shadow comparison counts +#: (``iac/shadow.py`` matches table, view, schema, ...), since no module holds the space. +PLAN_OP = "databricks.genie_space.converge" + + +def space_plan_action(space: Space) -> Dict[str, Any]: + """The space as a plan action: ``fluid plan`` lists it (``plan.py``, through the plugin's + ``planned_output_ports``) and ``fluid diff`` counts it among the resources the contract asks + for (``databricks.py::planned_objects``). + + No OpenTofu resource: ``fluid apply`` makes it through the Genie API after ``tofu apply``, + so no module or state holds it, and ``fluid diff``'s verdict does not read it (the live + check reads the tables, the state check the module's resources). Identified as the space + itself is, by the product's id, with no space id: none is known before an apply finds or + makes it. ``op`` and ``action_type`` both, as every plan action carries them. + """ + return { + "action_id": f"genie_space_{space.product_id}", + "op": PLAN_OP, + "action_type": PLAN_OP, + "provider": "databricks", + "resource_type": "databricks_genie_space", + "resource_id": space.product_id, + "description": f"Genie space {space.title!r}{_in(space)}: fluid apply creates or " + "updates it through the Genie API, after tofu apply", + "title": space.title, + "parent_path": space.parent_path or None, + "tables": list(space.tables), + "readers": list(space.readers), + } + + +def module_notice(contract: Mapping[str, Any]) -> Optional[str]: + """What ``fluid generate iac`` says about a contract that asks for a space: the module it + writes holds none, and a plain ``tofu apply`` of it makes none.""" + if not requested(contract): + return None + return ( + f"{_WHERE} asks for a Genie space, which this module does not hold: fluid apply " + "creates, updates or trashes it through the Genie API after tofu apply, so a plain " + "tofu apply of this module makes no space." + ) + + def _description(contract: Mapping[str, Any], product_id: str) -> str: about = _text(contract.get("description")) footer = ( diff --git a/tests/iac/test_iac_databricks_genie.py b/tests/iac/test_iac_databricks_genie.py index 220da99e..ad5a7d58 100644 --- a/tests/iac/test_iac_databricks_genie.py +++ b/tests/iac/test_iac_databricks_genie.py @@ -1415,3 +1415,92 @@ def test_dropping_the_block_leaves_a_space_in_the_parent_path_with_a_note(fake): run = Run(fake)(without_block(site_contract())) assert set(fake.genie_spaces) == {space_id} and run.writes() == [] assert f"left alone: space {space_id}" in run.text and SALES in run.text + + +# ---------------------------------------------------------------------- plan, diff, generate iac + + +def test_the_plan_lists_the_space_as_an_action_outside_the_module(monkeypatch): + from fluid_build.iac.shadow import native_logical_resources + + monkeypatch.setenv(dbx.WAREHOUSE_ENV, "wh-1") + doc = site_contract(parentPath=SALES) + (action,) = [ + a for a in dbx.planned_objects(doc) if a["resource_type"] == "databricks_genie_space" + ] + assert action == { + "action_id": f"genie_space_{PRODUCT}", + "op": "databricks.genie_space.converge", + "action_type": "databricks.genie_space.converge", + "provider": "databricks", + "resource_type": "databricks_genie_space", + "resource_id": PRODUCT, + "description": "Genie space 'Site 360' in /genie/sales: fluid apply creates or updates " + "it through the Genie API, after tofu apply", + "title": "Site 360", + "parent_path": SALES, + "tables": [TABLE], + "readers": list(space_of(doc).readers), + } + # No logical resource for the engines' parity check, and the module is the same with it. + assert native_logical_resources([action]) == set() + assert PLUGIN.emit(doc, [action]) == PLUGIN.emit(doc) + assert not [ + a + for a in dbx.planned_objects(without_block(doc)) + if a["resource_type"] == "databricks_genie_space" + ] + # fluid plan's hook lists the same action, and none for a contract the emitter refuses. + assert PLUGIN.planned_output_ports(doc) == [action] + assert PLUGIN.planned_output_ports(site_contract(parentPath="/Shared/x")) == [] + (own,) = [ + a + for a in dbx.planned_objects(site_contract()) + if a["resource_type"] == "databricks_genie_space" + ] + assert own["parent_path"] is None and " in " not in own["description"].split(":")[0] + + +def test_generate_iac_notes_the_space_its_module_does_not_hold(): + (notice,) = PLUGIN.standalone_module_notices(site_contract()) + assert "extensions.databricksGenie asks for a Genie space" in notice + assert "a plain tofu apply of this module makes no space" in notice + assert PLUGIN.standalone_module_notices(without_block(site_contract())) == [] + + +def test_the_cli_plans_lists_diffs_and_notes_the_space(monkeypatch, tmp_path, capfd): + from ._databricks_cli import isolate_workspace, make_contract, run_fluid, write_contract + + isolate_workspace( + monkeypatch, + tmp_path, + host="https://adb-1.azuredatabricks.net", + warehouse_id="wh-0123456789abcdef", + plugin_cache=str(tmp_path), + ) + doc = make_contract() + doc["extensions"] = {genie.EXTENSION_KEY: {"parentPath": SALES}} + path = write_contract(tmp_path, doc) + + rc, out = run_fluid(capfd, "generate", "iac", path, "--out", str(tmp_path / "iac")) + assert rc == 0, out + assert "Note: extensions.databricksGenie asks for a Genie space, which this module" in out + assert "databricks_genie" not in (tmp_path / "iac" / "main.tf.json").read_text() + + rc, out = run_fluid(capfd, "plan", path, "--out", str(tmp_path / "plan.json")) + assert rc == 0, out + plan = json.loads((tmp_path / "plan.json").read_text()) + (listed,) = [a for a in plan["actions"] if a.get("resource_type") == "databricks_genie_space"] + assert listed["resource_id"] == "dbx.fake.orders" and listed["parent_path"] == SALES + assert listed["step"] == plan["total_actions"] == len(plan["actions"]) + assert listed["action_id"] in plan["dependency_graph"]["nodes"] + assert f"genie_space_dbx.fake.orders ({genie.PLAN_OP})" in out + + # Listed among what the contract asks for, and no drift: nothing compares it with an apply. + rc, out = run_fluid( + capfd, "diff", path, "--exit-on-drift", "--out", str(tmp_path / "drift.json") + ) + assert rc == 0, out + report = json.loads((tmp_path / "drift.json").read_text()) + assert "databricks_genie_space:dbx.fake.orders" in report["changes"]["added"] + assert report["summary"]["has_drift"] is False From e4f3e97af48bb59ff008582fa09da4d6888c7ddc Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 23:41:31 +0100 Subject: [PATCH 3/7] fix(databricks): a Genie folder the account users group can edit is refused too Under identity federation the account's all-users group, account users, can be granted on a workspace folder. The parentPath folder check named only the workspace's own users group, so a folder that gave account users CAN_EDIT or CAN_MANAGE passed, and every account user could have edited the space or read its conversations. Both groups now count as every user, and the refusal names the group it found. --- AGENTS.md | 2 +- CHANGELOG.md | 3 +- HONESTLY_TESTED.md | 2 +- fluid_build/iac/providers/databricks_genie.py | 39 ++++++++++++------- tests/iac/test_iac_databricks_genie.py | 12 ++++++ 5 files changed, 42 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 11935c68..db61f844 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -374,7 +374,7 @@ made there and adopted only there, so the identities keep one space. One an earl in the identity's own folder is moved in, keeping its conversations. A space inherits its folder's permissions, so the folder is the trust boundary. `/Shared` is refused offline, because every workspace user holds `CAN_MANAGE` there. The apply also stops, before `tofu`, on a folder that -does not exist or one where the `users` group can edit or manage. Removing the block leaves a +does not exist or one where `users` or `account users` can edit or manage. Removing the block leaves a space in `parentPath` with a note naming it. `fluid plan` lists the space as a `databricks_genie_space` action, and `fluid generate iac` notes that its module holds none. Code: `fluid_build/iac/providers/databricks_genie.py`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4815ed66..d84618fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,7 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 boundary. `fluid validate` refuses anything under `/Shared`: every workspace user holds `CAN_MANAGE` there (measured), so each could read every reader's conversations. `fluid apply` stops before `tofu apply` when the folder is missing or not a folder, when its permissions - cannot be read, or when the `users` group holds `CAN_EDIT` or `CAN_MANAGE` on it. Removing the + cannot be read, or when a group of every user (`users`, or `account users` under identity + federation) holds `CAN_EDIT` or `CAN_MANAGE` on it. Removing the whole block leaves a space in `parentPath` in place, with a note naming it: without the block, the apply no longer knows which folder to trust. diff --git a/HONESTLY_TESTED.md b/HONESTLY_TESTED.md index 08bb9d9f..6c3f2eb7 100644 --- a/HONESTLY_TESTED.md +++ b/HONESTLY_TESTED.md @@ -94,7 +94,7 @@ Markers declared in `pyproject.toml`. | Manual live, Free Edition (founder-approved) | the real `fluid` CLI, no test file: run 2026-11-01 from main dfd6f145, databricks-sdk 0.158.0, every `fluid_iactest_*` object swept after (1 schema, 1 table, 1 folder; 0 left) | ✅ validate, `generate iac --validate`, plan, dry-run and apply `+9`; a second plan is EMPTY against the real Jobs API; Databricks accepts the Quartz schedule, serverless environment 2 with `dbt-databricks>=1.8.0,<2.0.0` (dbt 1.12.6, adapter 1.13.0), the WORKSPACE `project_directory` and the commands as emitted; one `run-now` ends SUCCESS and dbt builds the table; `fluid verify` with the real SDK (columns, types, one job); `fluid diff`'s state check (its live check has no Databricks inspector); a model change `~1`; the build made local removes job, files and folders `-5` without `--allow-data-loss` (the empty `.fluid//` folders above `files/` stay: only `files/` and below are managed). It found the USE CATALOG check reading a group-conveyed privilege as none. After the fix, `fluid verify` on a fresh one-table product (`+4`) passes every dimension, catalog access included (1 schema and 1 table swept, 0 left). | A GIT source, profile or OAuth auth, a run-as service principal, table adoption and a larger project, live; the region residency check (the metastore precondition and `fluid verify`'s residency dimension), which was added after this run and not run live; dbt's CREATE OR REPLACE drops a contract's NOT NULL, so verify flags a `required` column a dbt model does not constrain itself. | | Manual live, Free Edition (founder-approved), masks and row filters | the real `fluid` CLI, no test file: run 2026-11-03 at the tip of #785, a real workspace with an AWS us-west-2 metastore; every object swept after (0 left) | ✅ 10 steps passed. Functions are created and read back (owner is the user's email; `routine_definition` is the body without `RETURN`, backticks kept; `full_data_type` is STRING or BOOLEAN; a zero-parameter function's FunctionInfo has no `input_params`). The row filter and masks are bound and enforced on the restricted user's own SELECT: only the consented rows, `msisdn` as `XXXXX4567`, a 3-character value hashed, the denied column NULL. `SET ROW FILTER ... ON ()` is accepted and a `SET` replaces a live filter. Tags are present, `fluid verify` passes every dimension, a second apply is `+0 ~0 -0`, the residency precondition fails the plan and names both regions, `generate iac` refuses, and the sweep left 0. | Still open, as this run showed: superseded functions were left behind (orphans; the follow-up that drops them after a successful apply is not run live); `tofu destroy` of the module fails at a schema that still holds functions, so a teardown must drop them first; and the residency failure text was forge-cli's 800-character tail of the tofu JSON, which cut off the sentence naming the live region (the one-line explanation is not run live). A metastore in another cloud or region, a service principal as the run identity, and a group-conveyed grant on the live workspace are still not run. | | Manual live, Free Edition (founder-approved), 2026-11-04 | the real `fluid` CLI, no test file: #801 + #831 at a7169202, run by the demo session, every object swept back to the baseline after | ✅ Unity Catalog metadata read back live: the table and column comments, the product tags (`fluid_product_id` and the rest), the owner-team tag, the `fluid_sensitivity` column tags and `system.certification_status = certified` (accepted for an admin personal access token). The bronze landing: 5 rows loaded, the email column hashed, the staged files deleted from the volume; a second run overwrote the table (the Delta history shows `numRemovedFiles=1`). The column masks, the row filter and a scoped grant were enforced after the `INSERT OVERWRITE`. `lifecycle.retention: P30D` with `expire: true` was accepted, `fluid verify` passed, and predictive optimization was on. A second apply planned `+0 ~0 -0`. A changed row filter bound the new `fluid_rf_` function and dropped the superseded one. Findings fixed in the follow-up PR: the landing left the empty `///` directories and the local masked copy under `.fluid/staging//`, and a dry run did not list the superseded function drops. | Not tested live: `system.certification_status` set by a non-admin identity holding only ASSIGN on the governed tag (only an admin token was used). Open: `facets.databricks_load.rows` counts the rows loaded, which a row-filtered `SELECT COUNT(*)` by a restricted reader does not match (by design of the filter, not reconciled in the run record). | -| Stage 1 unit: Databricks Genie space (`extensions.databricksGenie`) | `tests/iac/test_iac_databricks_genie.py` (the compile, the plugin's `converge_output_ports` against the stand-in's Genie and Permissions APIs, the engine hook) | ✅ 131. What the space holds (sample questions split on `\|`, 32-hex ids sorted, column descriptions with allowed values, range, example and synonyms, one instruction with the metrics' SQL, `agentPolicy` and the access statement), prompt matching never on, a sensitive column's example left out, the refusals (`fluid validate` and before `tofu`), create + share then no change, an update with the `etag`, a 409 not overwritten, a hand edit set back, removal trashes, a stranger's space untouched, a space planted with the marker in another folder never adopted, shared or trashed, a marker quoted inside a description no match, two markers refused, an apply straight after a create finding the space (the list lags), a title another space has kept as the service suffixed it and a rename onto one said as such, a transient failure to read a space's folder failing the apply instead of making a second space, a new space outside the folder the next apply looks in trashed and said, a policy that allows `analysis` but not `qa` accepted and one that denies `analysis` refused, its lines printed literally (Rich read the marker and a `[/x]` title as markup), a principal granted `manage` (`ALL_PRIVILEGES`) counted as a reader, metric views counted toward the 50-entry cap, remediation by HTTP status, a missing SDK stopping the apply before `tofu`, the native plan refusing what `emit` refuses, one snippet per metric name, a principal the service keeps in another case not shared again, a dry run writes nothing, a metric view used once it exists and sent where the service keeps it, readers' `CAN_RUN` with editors kept, the real SDK's error classes read as 403/404/409, no credential in a message, the stand-in refusing what the service refuses, and measure SQL identical to the governed MCP query path's. `parentPath` (one shared folder): written as the API reads it back (no `/Workspace` prefix, no trailing `/`, case kept), a relative, root, `.`/`..` or non-string path and anything under `/Shared` refused offline, the space made in the folder and found there by a second identity, each identity keeping its own without it, a space in the identity's own folder moved in (dry run says so) and a copy left there trashed, two copies not guessed between, a marker space in the folder above, a folder inside, another case or someone's home left alone, a missing folder, a file, a folder `users` can edit or manage (directly or inherited) and unreadable folder permissions stopping the apply before `tofu`, `users` with `CAN_RUN` said, a create or move the service does not make stopping the apply, the block removed leaving the folder's space with a note; the `databricks_genie_space` plan action (`fluid plan`, `fluid diff`'s added list with no drift under `--exit-on-drift`, no shadow-parity kind, the module unchanged) and `fluid generate iac`'s note | Genie's answers (only the live row below asks one) | +| Stage 1 unit: Databricks Genie space (`extensions.databricksGenie`) | `tests/iac/test_iac_databricks_genie.py` (the compile, the plugin's `converge_output_ports` against the stand-in's Genie and Permissions APIs, the engine hook) | ✅ 132. What the space holds (sample questions split on `\|`, 32-hex ids sorted, column descriptions with allowed values, range, example and synonyms, one instruction with the metrics' SQL, `agentPolicy` and the access statement), prompt matching never on, a sensitive column's example left out, the refusals (`fluid validate` and before `tofu`), create + share then no change, an update with the `etag`, a 409 not overwritten, a hand edit set back, removal trashes, a stranger's space untouched, a space planted with the marker in another folder never adopted, shared or trashed, a marker quoted inside a description no match, two markers refused, an apply straight after a create finding the space (the list lags), a title another space has kept as the service suffixed it and a rename onto one said as such, a transient failure to read a space's folder failing the apply instead of making a second space, a new space outside the folder the next apply looks in trashed and said, a policy that allows `analysis` but not `qa` accepted and one that denies `analysis` refused, its lines printed literally (Rich read the marker and a `[/x]` title as markup), a principal granted `manage` (`ALL_PRIVILEGES`) counted as a reader, metric views counted toward the 50-entry cap, remediation by HTTP status, a missing SDK stopping the apply before `tofu`, the native plan refusing what `emit` refuses, one snippet per metric name, a principal the service keeps in another case not shared again, a dry run writes nothing, a metric view used once it exists and sent where the service keeps it, readers' `CAN_RUN` with editors kept, the real SDK's error classes read as 403/404/409, no credential in a message, the stand-in refusing what the service refuses, and measure SQL identical to the governed MCP query path's. `parentPath` (one shared folder): written as the API reads it back (no `/Workspace` prefix, no trailing `/`, case kept), a relative, root, `.`/`..` or non-string path and anything under `/Shared` refused offline, the space made in the folder and found there by a second identity, each identity keeping its own without it, a space in the identity's own folder moved in (dry run says so) and a copy left there trashed, two copies not guessed between, a marker space in the folder above, a folder inside, another case or someone's home left alone, a missing folder, a file, a folder `users` or `account users` can edit or manage (directly or inherited) and unreadable folder permissions stopping the apply before `tofu`, `users` with `CAN_RUN` said, a create or move the service does not make stopping the apply, the block removed leaving the folder's space with a note; the `databricks_genie_space` plan action (`fluid plan`, `fluid diff`'s added list with no drift under `--exit-on-drift`, no shadow-parity kind, the module unchanged) and `fluid generate iac`'s note | Genie's answers (only the live row below asks one) | | Stage 1 real tofu, stand-in workspace: Genie space | `tests/iac/test_iac_databricks_genie_fake_e2e.py` | ✅ 4. The real `fluid` CLI and `tofu`: validate, dry-run, apply (the space after the tables, shared with the readers), a second apply that changes nothing, the block removed (trashed); a Genie failure fails the apply after the tables with its fix and the next apply converges; an invalid block refused before `tofu`; `parentPath`: a missing folder stops the apply before `tofu` writes a table, then the space made in the folder and a second identity's apply changing nothing | The stand-in is modelled on the measured API, not the service itself | | Manual live, Free Edition, Genie space, 2026-11-12 | the real `fluid` CLI, no test file: branch `feat/databricks-genie-space`, databricks-sdk 0.162.0, a copy of industry-models tower-management `dp019_site_360` in the scratch catalog `fluid_iactest_genie` (no dbt build, no MENA sovereignty block: the workspace is in us-west-2), 6 synthetic rows | ✅ dry-run (would create, would share), apply `+73` then the space created and shared `CAN_RUN` with two persona service principals, a second apply `+0 ~0 -0` and "no change", the question "Which on-air sites in Amman are colocation-ready with hybrid power and no standing risk?" answered through the Genie Conversation API with the 2 expected sites (its SQL took the latest `snapshot_date` and the inverse of the `at_risk_site_count` filter from the instruction), a metric view created by hand picked up ("updated: metric views", then "no change"), the block removed (trashed). Two defects found and fixed in the run: the metric-view read failed on a schema not created yet, and the service keeps a metric view among the tables. A re-check after the security review (the plugin's own hook over `samples.nyctaxi.trips`, nothing created in Unity Catalog) found the list shows a new space only ~9 s later (a run straight after made a second space) and that a taken title is suffixed on create and 409 on PATCH; fixed, then create, an immediate second run ("no change", same space), and the block removed (trashed, via the folder check) passed live. Swept: the space trashed, `DROP CATALOG fluid_iactest_genie CASCADE`, 0 left | A reader without `SELECT` asking (no persona credential here; the access model is Genie's documented one); a service principal as the deploying identity (its folder is `/Users/`, the one Databricks Asset Bundles build from `current_user.userName`; a create that lands anywhere else is trashed and stops the apply); a space over several products | | Manual live, Free Edition, Genie `parentPath`, 2026-11-12 | the real `fluid` CLI, no test file: branch `feat/databricks-genie-shared-folder` on main 97811add, databricks-sdk 0.162.0, a one-table product in the scratch catalog `fluid_iactest_genie_pp`, its folder `/fluid_iactest_genie_pp` at the workspace root | ✅ A probe measured the API first. A create accepts `/Users/`, `/Workspace/Users/` (accepted this time; an earlier probe that day had a 400), `/Shared/x`, `/Workspace/Shared/x`, a trailing `/` and a root-level folder, and a GET returns the bare form. A missing folder is 404, a relative path 400, and `/shared/x` 404 (paths are case-sensitive). `/Shared` grants `users` `CAN_MANAGE` directly, and every folder and space under it inherits that; the root grants only admins. A space's permissions all come from its folder, and `PATCH {parent_path}` moves it. Then through the CLI: validate refuses `/Workspace/Shared/...`; an apply with the folder missing stops after `tofu plan`, before `tofu apply` (no schema made); a space made without `parentPath` in the user's folder, a dry run says it would move, the apply moves it (same id), and a second apply changes nothing; `users` with `CAN_MANAGE` on the folder is refused before `tofu apply`, and `users` with `CAN_RUN` is a note; `fluid plan` lists `genie_space_`, `generate iac` prints the note, and `diff --exit-on-drift` exits 0 with the space among `added`; with the block removed, the folder's space is left with a note. Swept: the space trashed, the folder deleted, the catalog dropped, 0 left | A second identity, live (no service-principal secret here; the stand-in e2e covers it); a folder a non-admin made inside someone's home | diff --git a/fluid_build/iac/providers/databricks_genie.py b/fluid_build/iac/providers/databricks_genie.py index 4baa50de..61b0b28d 100644 --- a/fluid_build/iac/providers/databricks_genie.py +++ b/fluid_build/iac/providers/databricks_genie.py @@ -111,6 +111,9 @@ FOLDER_PERMISSIONS_PATH = "/api/2.0/permissions/directories/{}" #: The group every workspace user is in, as the Permissions API names it. ALL_USERS = "users" +#: Every group that holds every user: the workspace's own, and the account's (granted on a +#: workspace object under identity federation). +ALL_USERS_GROUPS = (ALL_USERS, "account users") #: The folder that grants :data:`ALL_USERS` ``CAN_MANAGE`` directly; every folder and space under #: it inherits that (measured on a live workspace, 2026-11-12), and a workspace permission #: cannot be taken away below the folder that grants it ("objects in a folder inherit all @@ -981,8 +984,8 @@ def _at_most_one(spaces: Sequence[Mapping[str, Any]], space: Space) -> None: def _check_folder(client: Any, folder: str, say: Say) -> None: - """Stop unless ``folder`` is a workspace folder on which the :data:`ALL_USERS` group (every - workspace user) holds neither ``CAN_EDIT`` nor ``CAN_MANAGE``, directly or inherited. + """Stop unless ``folder`` is a workspace folder on which no group of every user + (:data:`ALL_USERS_GROUPS`) holds ``CAN_EDIT`` or ``CAN_MANAGE``, directly or inherited. A space inherits every permission on its folder (measured, 2026-11-12): whoever can edit the folder can edit the space's instructions, and whoever manages it can also read every @@ -997,33 +1000,43 @@ def _check_folder(client: Any, folder: str, say: Say) -> None: f"read the permissions on the folder {folder}", lambda: client.api_client.do("GET", FOLDER_PERMISSIONS_PATH.format(object_id)), ) - held = sorted(_group_levels(acl, ALL_USERS), key=lambda level: _LEVEL_RANK.get(level, 0)) - if held and held[-1] in _KEPT_LEVELS: + group, level = _broadest(acl) + if level in _KEPT_LEVELS: harm = "edit the space's instructions" - if held[-1] == "CAN_MANAGE": + if level == "CAN_MANAGE": harm = ( "edit the space's instructions, read every reader's conversations and make a " "space there that the apply would take for the product's" ) raise GenieError( - f"every workspace user holds {held[-1]} on the folder {folder} (the {ALL_USERS} " - f"group), and a Genie space inherits its folder's permissions: each of them could " - f"{harm}.", + f"every workspace user holds {level} on the folder {folder} (the {group} group), " + f"and a Genie space inherits its folder's permissions: each of them could {harm}.", ( - f"Take CAN_EDIT and CAN_MANAGE on {folder} away from {ALL_USERS}, or set " + f"Take CAN_EDIT and CAN_MANAGE on {folder} away from {group}, or set " f"{_WHERE}.parentPath to a folder that only the identities deploying the product " f"can edit or manage (not under {SHARED_FOLDER}, which grants {ALL_USERS} " "CAN_MANAGE).", ), ) - if held: + if level: say( - f" note: every workspace user holds {held[-1]} on {folder}, which the space " - "inherits: they can open it, and each answer still follows the asker's own Unity " - "Catalog grants." + f" note: every workspace user holds {level} on {folder} (the {group} group), " + "which the space inherits: they can open it, and each answer still follows the " + "asker's own Unity Catalog grants." ) +def _broadest(acl: Any) -> Tuple[str, str]: + """The group of every user holding the highest level on a folder, and that level (``""`` + for none of them).""" + best = (ALL_USERS, "") + for group in ALL_USERS_GROUPS: + for level in _group_levels(acl, group): + if _LEVEL_RANK.get(level, 0) > _LEVEL_RANK.get(best[1], 0): + best = (group, level) + return best + + def _folder_object_id(client: Any, folder: str) -> str: """The object id of the folder ``parentPath`` names, for the Permissions API path; a folder that does not exist, or a path that is no folder, stops the apply with what to do.""" diff --git a/tests/iac/test_iac_databricks_genie.py b/tests/iac/test_iac_databricks_genie.py index ad5a7d58..8f41beea 100644 --- a/tests/iac/test_iac_databricks_genie.py +++ b/tests/iac/test_iac_databricks_genie.py @@ -1371,6 +1371,18 @@ def test_a_parent_folder_every_user_can_open_is_said_not_refused(fake): assert only_space(fake)["parent_path"] == SALES +def test_the_account_users_group_counts_as_every_user(fake): + """Under identity federation the account's own all-users group can be granted on a folder.""" + in_sales(fake) + fake.folder_acl["/genie"] = {("group_name", "account users"): "CAN_MANAGE"} + run = Run(fake) + with pytest.raises(genie.GenieError) as exc: + run(site_contract(parentPath=SALES)) + assert "holds CAN_MANAGE on the folder /genie/sales (the account users group)" in str(exc.value) + assert "away from account users" in exc.value.remediation[0] + assert run.writes() == [] + + def test_folder_permissions_this_identity_cannot_read_stop_the_apply(fake): in_sales(fake) fake.fail_route("GET", "/api/2.0/permissions/directories/", 403, "no CAN_MANAGE") From f2b5de5b349a3429563994d9982fb877b690a3be Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 23:48:10 +0100 Subject: [PATCH 4/7] fix(databricks): Genie move and removal notes name the space and say why A move into parentPath now names the space by title and id, as the trash and left-alone lines do. Removing the block leaves a space in a parentPath folder, and the note for it now says why: without the block the apply cannot tell that folder from any other, so the product's own space there is to be trashed by hand. AGENTS.md's earlier sentence now names parentPath beside the identity's own folder. The workspace_folder docstring says the contract's value is trimmed by its caller, while API values are compared untrimmed. --- AGENTS.md | 4 +-- fluid_build/iac/providers/databricks_genie.py | 36 ++++++++++++------- tests/iac/test_iac_databricks_genie.py | 8 +++-- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index db61f844..37d445e9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -365,8 +365,8 @@ cases, purpose limitation). The space never widens access. Genie answers as the under their Unity Catalog grants. Prompt matching is never turned on. Only the principals with `SELECT` get `CAN RUN`. An `agentPolicy` that denies `qa` or `analysis`, or allows neither, refuses the space. The space is found again by the marker `[fluid-genie:]` at the end of its description, only in the -deploying identity's own folder (one anywhere else is left alone), and removing the block trashes -it. +deploying identity's own folder or in `parentPath` (one anywhere else is left alone), and removing +the block trashes a space in the identity's own folder. A product deployed from more than one identity (a laptop and CI) sets `parentPath`: a workspace folder that a workspace admin creates, with `CAN_MANAGE` only for those identities. The space is diff --git a/fluid_build/iac/providers/databricks_genie.py b/fluid_build/iac/providers/databricks_genie.py index 61b0b28d..e2357a3d 100644 --- a/fluid_build/iac/providers/databricks_genie.py +++ b/fluid_build/iac/providers/databricks_genie.py @@ -328,8 +328,8 @@ def workspace_folder(path: str) -> str: 2026-11-12). terraform-provider-databricks compares a dashboard's ``parent_path`` the same way (``common.WorkspacePathPrefixDiffSuppress``); here the prefix counts only as a whole segment. Workspace paths are case-sensitive (``/shared/x`` is not ``/Shared/x``: 404, - measured), so nothing is folded, and nothing is trimmed: a folder named ``"/x "`` is not - ``/x``.""" + measured), so nothing is folded, and nothing is trimmed: a folder the API names ``"/x "`` + is not ``/x``. (The contract's own value is trimmed first, by :func:`_block_parent_path`.)""" path = str(path or "") if path == "/Workspace" or path.startswith("/Workspace/"): path = path[len("/Workspace") :] @@ -946,8 +946,9 @@ def converge( serialized = _with_metric_views(client, space) space_id = str(found[0]["space_id"]) if found else "" if not found and own: - space_id = str(own.pop()["space_id"]) - _move(client, space_id, space, say, dry_run) + moving = own.pop() + space_id = str(moving["space_id"]) + _move(client, moving, space, say, dry_run) if space_id: _update_found(client, space_id, space, serialized, say, dry_run) elif dry_run: @@ -1080,13 +1081,16 @@ def _group_levels(doc: Any, group: str) -> set: return levels -def _move(client: Any, space_id: str, space: Space, say: Say, dry_run: bool) -> None: +def _move(client: Any, moving: Mapping[str, Any], space: Space, say: Say, dry_run: bool) -> None: """Move a space from this identity's own folder into ``parentPath`` (``PATCH parent_path``, measured 2026-11-12), so its id, link and conversations stay.""" + space_id = str(moving.get("space_id")) + what = ( + f"{moving.get('title')!r} ({space_id}) from this identity's own folder into " + f"{space.parent_path}" + ) if dry_run: - say( - f" would move space {space_id} from this identity's own folder into {space.parent_path}" - ) + say(f" would move {what}") return _call( client, @@ -1102,7 +1106,7 @@ def _move(client: Any, space_id: str, space: Space, say: Say, dry_run: bool) -> f"the space is in {parent or 'a folder this identity cannot read'}.", (f"Move it into {space.parent_path} by hand, or trash it, then re-run fluid apply.",), ) - say(f" moved space {space_id} from this identity's own folder into {space.parent_path}") + say(f" moved {what}") def _trash_copy( @@ -1308,7 +1312,7 @@ def _retire(product_id: str, say: Say, factory: Optional[ClientFactory], dry_run try: found = _find(client, marker(product_id)) if found: - found = _placed(client, found, say)[0] + found = _placed(client, found, say, hint=_RETIRED_HINT)[0] except Exception as exc: # noqa: BLE001 if _status(exc) not in (403, 404): say(f" note: not checked for a Genie space an earlier apply made: {_scrub(exc)}") @@ -1352,8 +1356,16 @@ def _find(client: Any, token: str) -> List[Dict[str, Any]]: query = {"page_size": _PAGE_SIZE, "page_token": token_next} +#: Said with each space a removed block leaves: the folder ``parentPath`` named is no longer +#: known, so the product's own space there looks like anyone's. +_RETIRED_HINT = ( + " If it is this product's space in its parentPath folder, trash it there: without " + f"extensions.{EXTENSION_KEY} this apply cannot tell that folder from any other." +) + + def _placed( - client: Any, found: List[Dict[str, Any]], say: Say, folder: str = "" + client: Any, found: List[Dict[str, Any]], say: Say, folder: str = "", hint: str = "" ) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]: """``found`` by folder: ``(the product's spaces, this identity's own copies)``. @@ -1392,7 +1404,7 @@ def _placed( f"{_marker_of(space)} but is in {parent or 'a folder this identity cannot read'}, " "not in " + (f"{folder} or " if folder else "") + "this identity's own folder, " "so it is not taken for the product's. Trash it, or take the marker out of its " - "description." + "description." + hint ) return product, own diff --git a/tests/iac/test_iac_databricks_genie.py b/tests/iac/test_iac_databricks_genie.py index 8f41beea..44c8806c 100644 --- a/tests/iac/test_iac_databricks_genie.py +++ b/tests/iac/test_iac_databricks_genie.py @@ -1270,11 +1270,14 @@ def test_a_space_in_this_identitys_own_folder_is_moved_into_the_parent_path(fake Run(fake)(site_contract()) space_id = only_space(fake)["space_id"] dry = Run(fake)(site_contract(parentPath=SALES), dry_run=True) - assert dry.writes() == [] and f"would move space {space_id}" in dry.text + assert dry.writes() == [] and f"would move 'Site 360' ({space_id}) from" in dry.text run = Run(fake)(site_contract(parentPath=SALES)) assert set(fake.genie_spaces) == {space_id} and only_space(fake)["parent_path"] == SALES assert run.writes()[0] == ("PATCH", f"/api/2.0/genie/spaces/{space_id}") - assert f"moved space {space_id} from this identity's own folder into /genie/sales" in run.text + assert ( + f"moved 'Site 360' ({space_id}) from this identity's own folder into /genie/sales" + in run.text + ) assert Run(fake)(site_contract(parentPath=SALES)).writes() == [] @@ -1427,6 +1430,7 @@ def test_dropping_the_block_leaves_a_space_in_the_parent_path_with_a_note(fake): run = Run(fake)(without_block(site_contract())) assert set(fake.genie_spaces) == {space_id} and run.writes() == [] assert f"left alone: space {space_id}" in run.text and SALES in run.text + assert "If it is this product's space in its parentPath folder, trash it there" in run.text # ---------------------------------------------------------------------- plan, diff, generate iac From a584d9bb0b7a24d8a6523e0f773a00211d8550bd Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Fri, 13 Nov 2026 00:04:24 +0100 Subject: [PATCH 5/7] fix(databricks): fluid plan reads the Genie space as fluid apply does; the folder check fails closed The defect scan found the plan's Genie action could differ from what the apply does: - It saw the contract's {{ env.* }} unresolved, so a reader was a placeholder, or the action dropped. - It had no contract directory, so a product with a local dbt build lost the action without a word. - It listed the space for every cloud the exposes name, while the apply runs one plugin. The plan hook now resolves the templates, takes the provider the apply's _resolve_provider picks, and runs inside contract_source. A contract the hook refuses is said as a warning with the refusal's kind. A planner that already lists the space keeps one listing. The parentPath folder check now fails closed on a permissions answer of another shape: an empty or missing access control list, an all-users entry without its levels, a level it does not know, a level in another case (compared upper-cased), or a folder answered without its object id. A move onto a title the folder already holds says so, with the fix, instead of the generic credential advice. --- AGENTS.md | 4 +- CHANGELOG.md | 6 +- HONESTLY_TESTED.md | 2 +- fluid_build/cli/plan.py | 86 +++++---- fluid_build/iac/providers/databricks.py | 16 +- fluid_build/iac/providers/databricks_genie.py | 104 ++++++---- tests/iac/_fake_databricks.py | 8 + tests/iac/test_iac_databricks_genie.py | 181 +++++++++++++++++- 8 files changed, 318 insertions(+), 89 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 37d445e9..e309c73b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -374,8 +374,8 @@ made there and adopted only there, so the identities keep one space. One an earl in the identity's own folder is moved in, keeping its conversations. A space inherits its folder's permissions, so the folder is the trust boundary. `/Shared` is refused offline, because every workspace user holds `CAN_MANAGE` there. The apply also stops, before `tofu`, on a folder that -does not exist or one where `users` or `account users` can edit or manage. Removing the block leaves a -space in `parentPath` with a note naming it. `fluid plan` lists the space as a +does not exist or one where `users` or `account users` can edit or manage. Removing the block, or +changing `parentPath`, leaves the space in the old folder with a note naming it. `fluid plan` lists the space as a `databricks_genie_space` action, and `fluid generate iac` notes that its module holds none. Code: `fluid_build/iac/providers/databricks_genie.py`. diff --git a/CHANGELOG.md b/CHANGELOG.md index d84618fb..730152c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,9 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `CAN_MANAGE` there (measured), so each could read every reader's conversations. `fluid apply` stops before `tofu apply` when the folder is missing or not a folder, when its permissions cannot be read, or when a group of every user (`users`, or `account users` under identity - federation) holds `CAN_EDIT` or `CAN_MANAGE` on it. Removing the - whole block leaves a space in `parentPath` in place, with a note naming it: without the block, - the apply no longer knows which folder to trust. + federation) holds `CAN_EDIT` or `CAN_MANAGE` on it. Removing the block, or removing or + changing `parentPath`, leaves the space in the old folder, with a note naming it: the apply + trusts only the folder the contract names now. - **`fluid plan` lists the Genie space** as a `databricks_genie_space` action with its title, folder, tables and readers. `fluid diff` counts it among the contract's resources and never diff --git a/HONESTLY_TESTED.md b/HONESTLY_TESTED.md index 6c3f2eb7..969032c5 100644 --- a/HONESTLY_TESTED.md +++ b/HONESTLY_TESTED.md @@ -94,7 +94,7 @@ Markers declared in `pyproject.toml`. | Manual live, Free Edition (founder-approved) | the real `fluid` CLI, no test file: run 2026-11-01 from main dfd6f145, databricks-sdk 0.158.0, every `fluid_iactest_*` object swept after (1 schema, 1 table, 1 folder; 0 left) | ✅ validate, `generate iac --validate`, plan, dry-run and apply `+9`; a second plan is EMPTY against the real Jobs API; Databricks accepts the Quartz schedule, serverless environment 2 with `dbt-databricks>=1.8.0,<2.0.0` (dbt 1.12.6, adapter 1.13.0), the WORKSPACE `project_directory` and the commands as emitted; one `run-now` ends SUCCESS and dbt builds the table; `fluid verify` with the real SDK (columns, types, one job); `fluid diff`'s state check (its live check has no Databricks inspector); a model change `~1`; the build made local removes job, files and folders `-5` without `--allow-data-loss` (the empty `.fluid//` folders above `files/` stay: only `files/` and below are managed). It found the USE CATALOG check reading a group-conveyed privilege as none. After the fix, `fluid verify` on a fresh one-table product (`+4`) passes every dimension, catalog access included (1 schema and 1 table swept, 0 left). | A GIT source, profile or OAuth auth, a run-as service principal, table adoption and a larger project, live; the region residency check (the metastore precondition and `fluid verify`'s residency dimension), which was added after this run and not run live; dbt's CREATE OR REPLACE drops a contract's NOT NULL, so verify flags a `required` column a dbt model does not constrain itself. | | Manual live, Free Edition (founder-approved), masks and row filters | the real `fluid` CLI, no test file: run 2026-11-03 at the tip of #785, a real workspace with an AWS us-west-2 metastore; every object swept after (0 left) | ✅ 10 steps passed. Functions are created and read back (owner is the user's email; `routine_definition` is the body without `RETURN`, backticks kept; `full_data_type` is STRING or BOOLEAN; a zero-parameter function's FunctionInfo has no `input_params`). The row filter and masks are bound and enforced on the restricted user's own SELECT: only the consented rows, `msisdn` as `XXXXX4567`, a 3-character value hashed, the denied column NULL. `SET ROW FILTER ... ON ()` is accepted and a `SET` replaces a live filter. Tags are present, `fluid verify` passes every dimension, a second apply is `+0 ~0 -0`, the residency precondition fails the plan and names both regions, `generate iac` refuses, and the sweep left 0. | Still open, as this run showed: superseded functions were left behind (orphans; the follow-up that drops them after a successful apply is not run live); `tofu destroy` of the module fails at a schema that still holds functions, so a teardown must drop them first; and the residency failure text was forge-cli's 800-character tail of the tofu JSON, which cut off the sentence naming the live region (the one-line explanation is not run live). A metastore in another cloud or region, a service principal as the run identity, and a group-conveyed grant on the live workspace are still not run. | | Manual live, Free Edition (founder-approved), 2026-11-04 | the real `fluid` CLI, no test file: #801 + #831 at a7169202, run by the demo session, every object swept back to the baseline after | ✅ Unity Catalog metadata read back live: the table and column comments, the product tags (`fluid_product_id` and the rest), the owner-team tag, the `fluid_sensitivity` column tags and `system.certification_status = certified` (accepted for an admin personal access token). The bronze landing: 5 rows loaded, the email column hashed, the staged files deleted from the volume; a second run overwrote the table (the Delta history shows `numRemovedFiles=1`). The column masks, the row filter and a scoped grant were enforced after the `INSERT OVERWRITE`. `lifecycle.retention: P30D` with `expire: true` was accepted, `fluid verify` passed, and predictive optimization was on. A second apply planned `+0 ~0 -0`. A changed row filter bound the new `fluid_rf_` function and dropped the superseded one. Findings fixed in the follow-up PR: the landing left the empty `///` directories and the local masked copy under `.fluid/staging//`, and a dry run did not list the superseded function drops. | Not tested live: `system.certification_status` set by a non-admin identity holding only ASSIGN on the governed tag (only an admin token was used). Open: `facets.databricks_load.rows` counts the rows loaded, which a row-filtered `SELECT COUNT(*)` by a restricted reader does not match (by design of the filter, not reconciled in the run record). | -| Stage 1 unit: Databricks Genie space (`extensions.databricksGenie`) | `tests/iac/test_iac_databricks_genie.py` (the compile, the plugin's `converge_output_ports` against the stand-in's Genie and Permissions APIs, the engine hook) | ✅ 132. What the space holds (sample questions split on `\|`, 32-hex ids sorted, column descriptions with allowed values, range, example and synonyms, one instruction with the metrics' SQL, `agentPolicy` and the access statement), prompt matching never on, a sensitive column's example left out, the refusals (`fluid validate` and before `tofu`), create + share then no change, an update with the `etag`, a 409 not overwritten, a hand edit set back, removal trashes, a stranger's space untouched, a space planted with the marker in another folder never adopted, shared or trashed, a marker quoted inside a description no match, two markers refused, an apply straight after a create finding the space (the list lags), a title another space has kept as the service suffixed it and a rename onto one said as such, a transient failure to read a space's folder failing the apply instead of making a second space, a new space outside the folder the next apply looks in trashed and said, a policy that allows `analysis` but not `qa` accepted and one that denies `analysis` refused, its lines printed literally (Rich read the marker and a `[/x]` title as markup), a principal granted `manage` (`ALL_PRIVILEGES`) counted as a reader, metric views counted toward the 50-entry cap, remediation by HTTP status, a missing SDK stopping the apply before `tofu`, the native plan refusing what `emit` refuses, one snippet per metric name, a principal the service keeps in another case not shared again, a dry run writes nothing, a metric view used once it exists and sent where the service keeps it, readers' `CAN_RUN` with editors kept, the real SDK's error classes read as 403/404/409, no credential in a message, the stand-in refusing what the service refuses, and measure SQL identical to the governed MCP query path's. `parentPath` (one shared folder): written as the API reads it back (no `/Workspace` prefix, no trailing `/`, case kept), a relative, root, `.`/`..` or non-string path and anything under `/Shared` refused offline, the space made in the folder and found there by a second identity, each identity keeping its own without it, a space in the identity's own folder moved in (dry run says so) and a copy left there trashed, two copies not guessed between, a marker space in the folder above, a folder inside, another case or someone's home left alone, a missing folder, a file, a folder `users` or `account users` can edit or manage (directly or inherited) and unreadable folder permissions stopping the apply before `tofu`, `users` with `CAN_RUN` said, a create or move the service does not make stopping the apply, the block removed leaving the folder's space with a note; the `databricks_genie_space` plan action (`fluid plan`, `fluid diff`'s added list with no drift under `--exit-on-drift`, no shadow-parity kind, the module unchanged) and `fluid generate iac`'s note | Genie's answers (only the live row below asks one) | +| Stage 1 unit: Databricks Genie space (`extensions.databricksGenie`) | `tests/iac/test_iac_databricks_genie.py` (the compile, the plugin's `converge_output_ports` against the stand-in's Genie and Permissions APIs, the engine hook) | ✅ 147. What the space holds (sample questions split on `\|`, 32-hex ids sorted, column descriptions with allowed values, range, example and synonyms, one instruction with the metrics' SQL, `agentPolicy` and the access statement), prompt matching never on, a sensitive column's example left out, the refusals (`fluid validate` and before `tofu`), create + share then no change, an update with the `etag`, a 409 not overwritten, a hand edit set back, removal trashes, a stranger's space untouched, a space planted with the marker in another folder never adopted, shared or trashed, a marker quoted inside a description no match, two markers refused, an apply straight after a create finding the space (the list lags), a title another space has kept as the service suffixed it and a rename onto one said as such, a transient failure to read a space's folder failing the apply instead of making a second space, a new space outside the folder the next apply looks in trashed and said, a policy that allows `analysis` but not `qa` accepted and one that denies `analysis` refused, its lines printed literally (Rich read the marker and a `[/x]` title as markup), a principal granted `manage` (`ALL_PRIVILEGES`) counted as a reader, metric views counted toward the 50-entry cap, remediation by HTTP status, a missing SDK stopping the apply before `tofu`, the native plan refusing what `emit` refuses, one snippet per metric name, a principal the service keeps in another case not shared again, a dry run writes nothing, a metric view used once it exists and sent where the service keeps it, readers' `CAN_RUN` with editors kept, the real SDK's error classes read as 403/404/409, no credential in a message, the stand-in refusing what the service refuses, and measure SQL identical to the governed MCP query path's. `parentPath` (one shared folder): written as the API reads it back (no `/Workspace` prefix, no trailing `/`, case kept), a relative, root, `.`/`..` or non-string path and anything under `/Shared` refused offline, the space made in the folder and found there by a second identity, each identity keeping its own without it, a space in the identity's own folder moved in (dry run says so) and a copy left there trashed, two copies not guessed between, a marker space in the folder above, a folder inside, another case or someone's home left alone, a missing folder, a file, a folder `users` or `account users` can edit or manage (directly or inherited) and unreadable folder permissions stopping the apply before `tofu`, `users` with `CAN_RUN` said, a create or move the service does not make stopping the apply, the block removed leaving the folder's space with a note; the `databricks_genie_space` plan action (`fluid plan`, `fluid diff`'s added list with no drift under `--exit-on-drift`, no shadow-parity kind, the module unchanged; the plan reading the contract as the apply does: `{{ env.* }}` resolved, only the provider the apply runs, a local dbt build's directory known; a refused contract said as a warning; listed once when a planner already holds it) and `fluid generate iac`'s note; a folder permissions answer of another shape, a level in another case or unknown, and a folder answered without its id stopping the apply; a move onto a title the folder holds said as such | Genie's answers (only the live row below asks one) | | Stage 1 real tofu, stand-in workspace: Genie space | `tests/iac/test_iac_databricks_genie_fake_e2e.py` | ✅ 4. The real `fluid` CLI and `tofu`: validate, dry-run, apply (the space after the tables, shared with the readers), a second apply that changes nothing, the block removed (trashed); a Genie failure fails the apply after the tables with its fix and the next apply converges; an invalid block refused before `tofu`; `parentPath`: a missing folder stops the apply before `tofu` writes a table, then the space made in the folder and a second identity's apply changing nothing | The stand-in is modelled on the measured API, not the service itself | | Manual live, Free Edition, Genie space, 2026-11-12 | the real `fluid` CLI, no test file: branch `feat/databricks-genie-space`, databricks-sdk 0.162.0, a copy of industry-models tower-management `dp019_site_360` in the scratch catalog `fluid_iactest_genie` (no dbt build, no MENA sovereignty block: the workspace is in us-west-2), 6 synthetic rows | ✅ dry-run (would create, would share), apply `+73` then the space created and shared `CAN_RUN` with two persona service principals, a second apply `+0 ~0 -0` and "no change", the question "Which on-air sites in Amman are colocation-ready with hybrid power and no standing risk?" answered through the Genie Conversation API with the 2 expected sites (its SQL took the latest `snapshot_date` and the inverse of the `at_risk_site_count` filter from the instruction), a metric view created by hand picked up ("updated: metric views", then "no change"), the block removed (trashed). Two defects found and fixed in the run: the metric-view read failed on a schema not created yet, and the service keeps a metric view among the tables. A re-check after the security review (the plugin's own hook over `samples.nyctaxi.trips`, nothing created in Unity Catalog) found the list shows a new space only ~9 s later (a run straight after made a second space) and that a taken title is suffixed on create and 409 on PATCH; fixed, then create, an immediate second run ("no change", same space), and the block removed (trashed, via the folder check) passed live. Swept: the space trashed, `DROP CATALOG fluid_iactest_genie CASCADE`, 0 left | A reader without `SELECT` asking (no persona credential here; the access model is Genie's documented one); a service principal as the deploying identity (its folder is `/Users/`, the one Databricks Asset Bundles build from `current_user.userName`; a create that lands anywhere else is trashed and stops the apply); a space over several products | | Manual live, Free Edition, Genie `parentPath`, 2026-11-12 | the real `fluid` CLI, no test file: branch `feat/databricks-genie-shared-folder` on main 97811add, databricks-sdk 0.162.0, a one-table product in the scratch catalog `fluid_iactest_genie_pp`, its folder `/fluid_iactest_genie_pp` at the workspace root | ✅ A probe measured the API first. A create accepts `/Users/`, `/Workspace/Users/` (accepted this time; an earlier probe that day had a 400), `/Shared/x`, `/Workspace/Shared/x`, a trailing `/` and a root-level folder, and a GET returns the bare form. A missing folder is 404, a relative path 400, and `/shared/x` 404 (paths are case-sensitive). `/Shared` grants `users` `CAN_MANAGE` directly, and every folder and space under it inherits that; the root grants only admins. A space's permissions all come from its folder, and `PATCH {parent_path}` moves it. Then through the CLI: validate refuses `/Workspace/Shared/...`; an apply with the folder missing stops after `tofu plan`, before `tofu apply` (no schema made); a space made without `parentPath` in the user's folder, a dry run says it would move, the apply moves it (same id), and a second apply changes nothing; `users` with `CAN_MANAGE` on the folder is refused before `tofu apply`, and `users` with `CAN_RUN` is a note; `fluid plan` lists `genie_space_`, `generate iac` prints the note, and `diff --exit-on-drift` exits 0 with the space among `added`; with the block removed, the folder's space is left with a note. Swept: the space trashed, the folder deleted, the catalog dropped, 0 left | A second identity, live (no service-principal secret here; the stand-in e2e covers it); a folder a non-admin made inside someone's home | diff --git a/fluid_build/cli/plan.py b/fluid_build/cli/plan.py index 822b7905..28333b95 100644 --- a/fluid_build/cli/plan.py +++ b/fluid_build/cli/plan.py @@ -354,7 +354,7 @@ def run(args, logger: logging.Logger) -> int: # What the apply makes outside OpenTofu (a Databricks Genie space), before the digests. if not streaming: - _append_output_port_actions(plan, contract, logger) + _append_output_port_actions(plan, contract, args, logger) # --- Packaging truthfulness (RFC-packaging-modes.md file 8) --------- # plan.json is the digest-bound artifact the human reviews, and @@ -863,50 +863,62 @@ def _validate_plan_actions(plan: Dict[str, Any], logger: logging.Logger) -> None def _append_output_port_actions( - plan: Dict[str, Any], contract: Dict[str, Any], logger: logging.Logger + plan: Dict[str, Any], contract: Dict[str, Any], args, logger: logging.Logger ) -> None: """Add to ``plan`` the actions an IaC plugin names for what ``fluid apply`` makes after ``tofu apply`` (its ``planned_output_ports`` hook, beside ``converge_output_ports``): the - Databricks plugin's Genie space. One per platform the contract's exposes bind to, from the - in-tree plugins only (``IAC_PLUGINS``: nothing is loaded from an entry point to plan). They - come last and depend on nothing in the graph, since they run after the whole tofu apply. - The OpenTofu engine applies from the contract, never from plan actions, so they change what - is reviewed, not what is applied; ``planDigest`` covers them. Never on a managed streaming - plan, whose apply runs no OpenTofu and checks its actions against its artifacts one for one. - A hook that fails leaves its actions out with a warning: ``fluid validate`` and ``fluid - apply`` refuse the contract with the reason, and the plan of the rest stays readable.""" - from fluid_build.iac.provider_match import canonical_cloud - - platforms = sorted( - { - canonical_cloud((expose.get("binding") or {}).get("platform")) - for expose in contract.get("exposes") or [] - if isinstance(expose, dict) and isinstance(expose.get("binding"), dict) - } - - {""} - ) - if not platforms: - return + Databricks plugin's Genie space. + + The hook sees the contract as the apply reads it: its ``{{ env.* }}`` resolved, the one + provider ``_resolve_provider`` picks (``--provider``, or the contract's only cloud: an + ambiguous or mismatched one lists nothing, since the apply refuses it too), and the + contract's directory known, where a local dbt build's project is read from. In-tree plugins + only (``IAC_PLUGINS``: nothing is loaded from an entry point to plan). The actions come last + and depend on nothing in the graph, since they run after the whole tofu apply; one a planner + already listed (the native ``provider.plan``) is kept once. The OpenTofu engine applies from + the contract, never from plan actions, so they change what is reviewed, not what is applied; + ``planDigest`` covers them. Never on a managed streaming plan, whose apply runs no OpenTofu + and checks its actions against its artifacts one for one. A hook that fails leaves its + actions out with a warning: ``fluid validate`` and ``fluid apply`` refuse the contract with + the reason, and the plan of the rest stays readable.""" + import copy + # Function-local: ``iac`` pulls every provider plugin, off the ``fluid --help`` cold path. from fluid_build.iac import IAC_PLUGINS + from fluid_build.iac.source import contract_source, source_contract_file - added: List[Dict[str, Any]] = [] - for name in platforms: - hook = getattr(IAC_PLUGINS.get(name), "planned_output_ports", None) - if not callable(hook): - continue - try: - added.extend(dict(action) for action in hook(contract)) - except Exception as exc: # noqa: BLE001 - the plan of everything else still stands - warn( - logger, - "plan_output_ports_skipped", - provider=name, - error=f"{type(exc).__name__}: {exc}", - ) - if not added: + from ._common import resolve_env_templates_in_contract + from .generate_iac import _resolve_provider + + try: + resolved = resolve_env_templates_in_contract(copy.deepcopy(dict(contract))) + except Exception as exc: # noqa: BLE001 - the plan of everything else still stands + warn(logger, "plan_output_ports_skipped", error=f"{type(exc).__name__}: {exc}") + return + try: + provider = _resolve_provider(resolved, getattr(args, "provider", None) or "auto") + except CLIError: + return + hook = getattr(IAC_PLUGINS.get(provider), "planned_output_ports", None) + if not callable(hook): + return + try: + with contract_source(source_contract_file(getattr(args, "contract", None))): + added = [dict(action) for action in hook(resolved)] + except Exception as exc: # noqa: BLE001 - the plan of everything else still stands + warn( + logger, + "plan_output_ports_skipped", + provider=provider, + kind=getattr(exc, "kind", None), + error=f"{type(exc).__name__}: {exc}", + ) return actions = list(plan.get("actions") or []) + present = {a.get("action_id") for a in actions if isinstance(a, dict)} + added = [action for action in added if action.get("action_id") not in present] + if not added: + return for action in added: actions.append({"step": len(actions) + 1, "depends_on": [], **action}) plan["actions"] = actions diff --git a/fluid_build/iac/providers/databricks.py b/fluid_build/iac/providers/databricks.py index 9f6c9665..7f664d75 100644 --- a/fluid_build/iac/providers/databricks.py +++ b/fluid_build/iac/providers/databricks.py @@ -2358,15 +2358,19 @@ def standalone_module_refusal( def planned_output_ports(self, contract: Mapping[str, Any]) -> List[Dict[str, Any]]: """``fluid plan``'s actions for what :meth:`converge_output_ports` makes after ``tofu apply``: the Genie space ``extensions.databricksGenie`` asks for - (``databricks_genie.space_plan_action``). A contract the emitter refuses lists none here; - ``fluid validate`` and ``fluid apply`` say why.""" - from .databricks_genie import space_plan_action + (``databricks_genie.space_plan_action``). A contract that asks for one and that the + emitter refuses raises the refusal, which ``fluid plan`` says as a warning.""" + from .databricks_genie import requested, space_plan_action + if not requested(contract): + return [] compiled = _compile(contract, os.environ) if compiled.refusals: - return [] - space = _genie(contract, compiled, os.environ).space - return [space_plan_action(space)] if space is not None else [] + raise compiled.refusals[0] + genie = _genie(contract, compiled, os.environ) + if genie.refusals: + raise genie.refusals[0] + return [space_plan_action(genie.space)] if genie.space is not None else [] def standalone_module_notices(self, contract: Mapping[str, Any]) -> List[str]: """What a module ``fluid generate iac`` writes leaves out that ``fluid apply`` makes: diff --git a/fluid_build/iac/providers/databricks_genie.py b/fluid_build/iac/providers/databricks_genie.py index e2357a3d..21afb6e4 100644 --- a/fluid_build/iac/providers/databricks_genie.py +++ b/fluid_build/iac/providers/databricks_genie.py @@ -1001,7 +1001,7 @@ def _check_folder(client: Any, folder: str, say: Say) -> None: f"read the permissions on the folder {folder}", lambda: client.api_client.do("GET", FOLDER_PERMISSIONS_PATH.format(object_id)), ) - group, level = _broadest(acl) + group, level = _broadest(acl, folder) if level in _KEPT_LEVELS: harm = "edit the space's instructions" if level == "CAN_MANAGE": @@ -1027,14 +1027,40 @@ def _check_folder(client: Any, folder: str, say: Say) -> None: ) -def _broadest(acl: Any) -> Tuple[str, str]: - """The group of every user holding the highest level on a folder, and that level (``""`` - for none of them).""" +def _broadest(acl: Any, folder: str) -> Tuple[str, str]: + """The group of every user holding the highest level on ``folder``, and that level (``""`` + for none of them). Any other answer than the measured shape stops the apply: every folder + lists at least ``admins`` (measured), so an empty list, an entry without its levels or a + level this module does not know could hide a grant to every user.""" + entries = acl.get("access_control_list") if isinstance(acl, Mapping) else None + if not isinstance(entries, list) or not entries: + raise GenieError( + f"the permissions on the folder {folder} came back without an access control list, " + "so whether every workspace user can edit or manage it is unknown.", + (_remediation(None, f"read the permissions on the folder {folder}"),), + ) best = (ALL_USERS, "") - for group in ALL_USERS_GROUPS: - for level in _group_levels(acl, group): - if _LEVEL_RANK.get(level, 0) > _LEVEL_RANK.get(best[1], 0): - best = (group, level) + for entry in entries: + group = entry.get("group_name") if isinstance(entry, Mapping) else None + if group not in ALL_USERS_GROUPS: + continue + held = entry.get("all_permissions") + levels = [ + str(item.get("permission_level") or "").upper() + for item in (held if isinstance(held, list) else ()) + if isinstance(item, Mapping) + ] + unknown = [level for level in levels if level not in _LEVEL_RANK] + if not levels or unknown: + raise GenieError( + f"the permissions on the folder {folder} do not say plainly what the {group} " + f"group holds ({unknown or held!r}), so whether every workspace user can edit " + "or manage it is unknown.", + (f"Check the permissions on {folder}, then re-run fluid apply.",), + ) + for level in levels: + if _LEVEL_RANK[level] > _LEVEL_RANK.get(best[1], 0): + best = (str(group), level) return best @@ -1066,19 +1092,14 @@ def _folder_object_id(client: Any, folder: str) -> str: + ", and a Genie space goes in a folder.", (f"Set {where} to a workspace folder.",), ) - return quote(str(status.get("object_id"))) - - -def _group_levels(doc: Any, group: str) -> set: - """Every level ``group`` holds in a Permissions API document, direct or inherited.""" - levels: set = set() - entries = doc.get("access_control_list") if isinstance(doc, Mapping) else None - for entry in entries or (): - if isinstance(entry, Mapping) and entry.get("group_name") == group: - for held in entry.get("all_permissions") or (): - if isinstance(held, Mapping) and held.get("permission_level"): - levels.add(str(held["permission_level"])) - return levels + object_id = status.get("object_id") + if object_id is None or str(object_id) == "": + raise GenieError( + f"the workspace answered for the folder {folder} without its object id, so its " + "permissions cannot be read.", + (f"Check that {folder} is a workspace folder, then re-run fluid apply.",), + ) + return quote(str(object_id)) def _move(client: Any, moving: Mapping[str, Any], space: Space, say: Say, dry_run: bool) -> None: @@ -1092,13 +1113,26 @@ def _move(client: Any, moving: Mapping[str, Any], space: Space, say: Say, dry_ru if dry_run: say(f" would move {what}") return - _call( - client, - f"move the Genie space into {space.parent_path}", - lambda: client.api_client.do( + try: + client.api_client.do( "PATCH", f"{SPACES_PATH}/{quote(space_id)}", body={"parent_path": space.parent_path} - ), - ) + ) + except Exception as exc: # noqa: BLE001 — a taken title is said as such + if _status(exc) == 409: + raise GenieError( + f"could not move space {space_id} into {space.parent_path}: another space there " + f"is already called {moving.get('title')!r}, and Genie keeps titles unique in a " + "folder (measured for a create and a rename).", + ( + f"Give this space a title of its own with extensions.{EXTENSION_KEY}.title, " + "or rename the other space, then re-run fluid apply.", + ), + ) from exc + raise GenieError( + f"could not move the Genie space into {space.parent_path}: " + f"{type(exc).__name__}: {_scrub(exc)}", + (_remediation(_status(exc), f"move the Genie space into {space.parent_path}"),), + ) from exc parent = _call(client, "read the moved Genie space", lambda: _parent(client, space_id)) if workspace_folder(parent) != space.parent_path: raise GenieError( @@ -1312,7 +1346,7 @@ def _retire(product_id: str, say: Say, factory: Optional[ClientFactory], dry_run try: found = _find(client, marker(product_id)) if found: - found = _placed(client, found, say, hint=_RETIRED_HINT)[0] + found = _placed(client, found, say)[0] except Exception as exc: # noqa: BLE001 if _status(exc) not in (403, 404): say(f" note: not checked for a Genie space an earlier apply made: {_scrub(exc)}") @@ -1356,16 +1390,8 @@ def _find(client: Any, token: str) -> List[Dict[str, Any]]: query = {"page_size": _PAGE_SIZE, "page_token": token_next} -#: Said with each space a removed block leaves: the folder ``parentPath`` named is no longer -#: known, so the product's own space there looks like anyone's. -_RETIRED_HINT = ( - " If it is this product's space in its parentPath folder, trash it there: without " - f"extensions.{EXTENSION_KEY} this apply cannot tell that folder from any other." -) - - def _placed( - client: Any, found: List[Dict[str, Any]], say: Say, folder: str = "", hint: str = "" + client: Any, found: List[Dict[str, Any]], say: Say, folder: str = "" ) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]: """``found`` by folder: ``(the product's spaces, this identity's own copies)``. @@ -1404,7 +1430,9 @@ def _placed( f"{_marker_of(space)} but is in {parent or 'a folder this identity cannot read'}, " "not in " + (f"{folder} or " if folder else "") + "this identity's own folder, " "so it is not taken for the product's. Trash it, or take the marker out of its " - "description." + hint + "description. If it is this product's own space, left in a folder the contract no " + f"longer names as {_WHERE}.parentPath, trash it there by hand: the apply trusts no " + "other folder." ) return product, own diff --git a/tests/iac/_fake_databricks.py b/tests/iac/_fake_databricks.py index a513764b..6add884c 100644 --- a/tests/iac/_fake_databricks.py +++ b/tests/iac/_fake_databricks.py @@ -1728,6 +1728,14 @@ def _genie_patch(self, m: Any, q: Any, b: Any) -> Tuple[int, Any]: b["title"], space["parent_path"], other_than=space_id ): return _err(409, "ALREADY_EXISTS", f"A space named {b['title']} already exists.") + # Titles are unique in a folder (measured for a create and a rename); a move onto a + # taken one is answered as the rename is (assumed, not measured). + if "parent_path" in b and self._genie_title_taken( + b.get("title", space["title"]), b["parent_path"], other_than=space_id + ): + return _err( + 409, "ALREADY_EXISTS", f"A space named {space['title']} already exists." + ) for key in ("title", "description", "warehouse_id", "parent_path"): if key in b: space[key] = b[key] diff --git a/tests/iac/test_iac_databricks_genie.py b/tests/iac/test_iac_databricks_genie.py index 44c8806c..b367ce6e 100644 --- a/tests/iac/test_iac_databricks_genie.py +++ b/tests/iac/test_iac_databricks_genie.py @@ -18,6 +18,7 @@ import copy import json +import logging from typing import Any, Dict, Iterator, List import pytest @@ -1386,6 +1387,74 @@ def test_the_account_users_group_counts_as_every_user(fake): assert run.writes() == [] +class _FolderClient: + """Answers the two folder reads with canned documents, as an odd service might.""" + + def __init__(self, status: Any, acl: Any): + self.api_client = self + self._status, self._acl = status, acl + self.paths: List[str] = [] + + def do(self, method: str, path: str, query: Any = None, body: Any = None) -> Any: + self.paths.append(path) + return self._status if path == genie.WORKSPACE_STATUS_PATH else self._acl + + +_DIR = {"object_type": "DIRECTORY", "object_id": 5} +_ADMINS = {"group_name": "admins", "all_permissions": [{"permission_level": "CAN_MANAGE"}]} + + +@pytest.mark.parametrize( + "acl", + [ + None, + [], + {}, + {"access_control_list": []}, + {"access_control_list": [_ADMINS, {"group_name": "users"}]}, + {"access_control_list": [_ADMINS, {"group_name": "users", "all_permissions": None}]}, + {"access_control_list": [_ADMINS, {"group_name": "users", "all_permissions": ["x"]}]}, + { + "access_control_list": [ + _ADMINS, + {"group_name": "users", "all_permissions": [{"permission_level": "CAN_OWN"}]}, + ] + }, + ], +) +def test_folder_permissions_of_another_shape_stop_the_apply(acl): + """Every folder lists admins at least: an answer that says less could hide a grant.""" + with pytest.raises(genie.GenieError): + genie._check_folder(_FolderClient(_DIR, acl), SALES, lambda _line: None) + + +def test_a_level_in_another_case_counts_and_a_folder_without_its_id_stops(): + acl = { + "access_control_list": [ + _ADMINS, + {"group_name": "users", "all_permissions": [{"permission_level": "can_manage"}]}, + ] + } + with pytest.raises(genie.GenieError) as exc: + genie._check_folder(_FolderClient(_DIR, acl), SALES, lambda _line: None) + assert "holds CAN_MANAGE" in str(exc.value) + no_id = _FolderClient({"object_type": "DIRECTORY"}, {"access_control_list": [_ADMINS]}) + with pytest.raises(genie.GenieError) as exc: + genie._check_folder(no_id, SALES, lambda _line: None) + assert "without its object id" in str(exc.value) + assert not any("None" in path for path in no_id.paths) + + +def test_a_move_onto_a_title_the_folder_holds_says_so(fake): + in_sales(fake) + Run(fake)(site_contract()) + fake.add_genie_space("Site 360", "another product's", parent_path=SALES) + with pytest.raises(genie.GenieError) as exc: + Run(fake)(site_contract(parentPath=SALES)) + assert "another space there is already called 'Site 360'" in str(exc.value) + assert "title of its own" in exc.value.remediation[0] + + def test_folder_permissions_this_identity_cannot_read_stop_the_apply(fake): in_sales(fake) fake.fail_route("GET", "/api/2.0/permissions/directories/", 403, "no CAN_MANAGE") @@ -1430,7 +1499,7 @@ def test_dropping_the_block_leaves_a_space_in_the_parent_path_with_a_note(fake): run = Run(fake)(without_block(site_contract())) assert set(fake.genie_spaces) == {space_id} and run.writes() == [] assert f"left alone: space {space_id}" in run.text and SALES in run.text - assert "If it is this product's space in its parentPath folder, trash it there" in run.text + assert "left in a folder the contract no longer names as" in run.text # ---------------------------------------------------------------------- plan, diff, generate iac @@ -1468,7 +1537,9 @@ def test_the_plan_lists_the_space_as_an_action_outside_the_module(monkeypatch): ] # fluid plan's hook lists the same action, and none for a contract the emitter refuses. assert PLUGIN.planned_output_ports(doc) == [action] - assert PLUGIN.planned_output_ports(site_contract(parentPath="/Shared/x")) == [] + assert PLUGIN.planned_output_ports(without_block(doc)) == [] + with pytest.raises(UnsupportedBindingError): # fluid plan says it as a warning + PLUGIN.planned_output_ports(site_contract(parentPath="/Shared/x")) (own,) = [ a for a in dbx.planned_objects(site_contract()) @@ -1477,6 +1548,112 @@ def test_the_plan_lists_the_space_as_an_action_outside_the_module(monkeypatch): assert own["parent_path"] is None and " in " not in own["description"].split(":")[0] +def _plan_args(**kw: Any) -> Any: + from types import SimpleNamespace + + return SimpleNamespace(**{"provider": None, "contract": None, **kw}) + + +def _planned_spaces(doc: Dict[str, Any], **kw: Any) -> List[Dict[str, Any]]: + from fluid_build.cli import plan as plan_cli + + plan: Dict[str, Any] = {"actions": []} + plan_cli._append_output_port_actions(plan, doc, _plan_args(**kw), logging.getLogger("t")) + return [a for a in plan["actions"] if a.get("resource_type") == "databricks_genie_space"] + + +def test_the_plan_reads_the_contract_as_the_apply_does(monkeypatch): + """Its {{ env.* }} resolved: the readers are who the apply shares the space with.""" + monkeypatch.setenv(dbx.WAREHOUSE_ENV, "wh-1") + monkeypatch.setenv("FLUID_TEST_NOC", "noc@towers.io") + doc = site_contract() + doc["exposes"][0]["binding"]["principals"][NOC] = "{{ env.FLUID_TEST_NOC }}" + (listed,) = _planned_spaces(doc) + assert "noc@towers.io" in listed["readers"] + assert not any("{{" in reader for reader in listed["readers"]) + + +def test_the_plan_lists_the_space_only_for_the_provider_the_apply_runs(monkeypatch): + monkeypatch.setenv(dbx.WAREHOUSE_ENV, "wh-1") + doc = site_contract() + doc["exposes"].append( + { + "exposeId": "events", + "kind": "table", + "binding": { + "platform": "gcp", + "format": "bigquery_table", + "location": {"project": "p-1", "dataset": "d", "table": "events"}, + }, + "contract": {"schema": [{"name": "id", "type": "STRING"}]}, + } + ) + assert len(_planned_spaces(doc, provider="databricks")) == 1 + assert _planned_spaces(doc, provider="gcp") == [] + assert _planned_spaces(doc) == [] # two clouds: the apply needs --provider too + + +def test_a_refused_contract_is_said_by_the_plan_not_dropped(monkeypatch, caplog): + monkeypatch.setenv(dbx.WAREHOUSE_ENV, "wh-1") + with caplog.at_level(logging.WARNING): + assert _planned_spaces(site_contract(parentPath="/Shared/x")) == [] + assert "plan_output_ports_skipped" in caplog.text and "databricks-genie-folder" in caplog.text + + +def test_a_plan_that_already_holds_the_space_does_not_list_it_twice(monkeypatch): + """The native planner (``provider.plan``, the path a plan without the action parser takes) + lists the space through ``planned_objects`` already.""" + from fluid_build.cli import plan as plan_cli + + monkeypatch.setenv(dbx.WAREHOUSE_ENV, "wh-1") + doc = site_contract(parentPath=SALES) + plan = {"actions": dbx.planned_objects(doc)} + before = len(plan["actions"]) + plan_cli._append_output_port_actions(plan, doc, _plan_args(), logging.getLogger("t")) + assert len(plan["actions"]) == before + assert [a for a in plan["actions"] if a["resource_type"] == "databricks_genie_space"] + + +def test_the_plan_lists_the_space_of_a_product_with_a_local_dbt_build(monkeypatch, tmp_path, capfd): + """A local dbt project is read from the contract's directory, which the plan must know.""" + from ._databricks_cli import isolate_workspace, make_contract, run_fluid, write_contract + + isolate_workspace( + monkeypatch, + tmp_path, + host="https://adb-1.azuredatabricks.net", + warehouse_id="wh-0123456789abcdef", + plugin_cache=str(tmp_path), + ) + project = tmp_path / "dbt" + (project / "models").mkdir(parents=True) + (project / "dbt_project.yml").write_text("name: p\n", encoding="utf-8") + (project / "models" / "orders.sql").write_text("select 1 as id", encoding="utf-8") + doc = make_contract() + doc["builds"] = [ + { + "id": "dbt_daily", + "engine": "dbt", + "pattern": "hybrid-reference", + "repository": "./dbt", + "properties": {"model": "orders", "select": "orders+"}, + "outputs": ["orders"], + "execution": { + "trigger": {"type": "schedule", "cron": "0 2 * * *"}, + "runtime": {"platform": "databricks"}, + }, + } + ] + doc["extensions"] = {genie.EXTENSION_KEY: {}} + path = write_contract(tmp_path, doc) + rc, out = run_fluid(capfd, "plan", path, "--out", str(tmp_path / "plan.json")) + assert rc == 0, out + actions = json.loads((tmp_path / "plan.json").read_text())["actions"] + assert [ + a["resource_id"] for a in actions if a.get("resource_type") == "databricks_genie_space" + ] == ["dbx.fake.orders"] + + def test_generate_iac_notes_the_space_its_module_does_not_hold(): (notice,) = PLUGIN.standalone_module_notices(site_contract()) assert "extensions.databricksGenie asks for a Genie space" in notice From d51a4c19f8dd5c0925a51a83b8849890b29c3f6d Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Fri, 13 Nov 2026 00:09:50 +0100 Subject: [PATCH 6/7] refactor(databricks): split the Genie folder-permission parse and the plan hook Structure only, no behaviour change. _broadest now reads the entries, and two helpers take over the rest: _held_levels parses and refuses one all-users entry, and _unreadable builds the fail-closed error. The plan hook's merge into plan.json stays in _append_output_port_actions, while _output_port_actions reads the contract as the apply does. ripwire's quality delta had the first at complexity 20 against a bar of 15, and the second at 64 lines against 60. --- fluid_build/cli/plan.py | 72 +++++++++++-------- fluid_build/iac/providers/databricks_genie.py | 49 +++++++------ 2 files changed, 70 insertions(+), 51 deletions(-) diff --git a/fluid_build/cli/plan.py b/fluid_build/cli/plan.py index 28333b95..d457befd 100644 --- a/fluid_build/cli/plan.py +++ b/fluid_build/cli/plan.py @@ -867,20 +867,42 @@ def _append_output_port_actions( ) -> None: """Add to ``plan`` the actions an IaC plugin names for what ``fluid apply`` makes after ``tofu apply`` (its ``planned_output_ports`` hook, beside ``converge_output_ports``): the - Databricks plugin's Genie space. - - The hook sees the contract as the apply reads it: its ``{{ env.* }}`` resolved, the one - provider ``_resolve_provider`` picks (``--provider``, or the contract's only cloud: an - ambiguous or mismatched one lists nothing, since the apply refuses it too), and the - contract's directory known, where a local dbt build's project is read from. In-tree plugins - only (``IAC_PLUGINS``: nothing is loaded from an entry point to plan). The actions come last - and depend on nothing in the graph, since they run after the whole tofu apply; one a planner - already listed (the native ``provider.plan``) is kept once. The OpenTofu engine applies from - the contract, never from plan actions, so they change what is reviewed, not what is applied; - ``planDigest`` covers them. Never on a managed streaming plan, whose apply runs no OpenTofu - and checks its actions against its artifacts one for one. A hook that fails leaves its - actions out with a warning: ``fluid validate`` and ``fluid apply`` refuse the contract with - the reason, and the plan of the rest stays readable.""" + Databricks plugin's Genie space (:func:`_output_port_actions`). + + They come last and depend on nothing in the graph, since they run after the whole tofu + apply; one a planner already listed (the native ``provider.plan``) is kept once. The + OpenTofu engine applies from the contract, never from plan actions, so they change what is + reviewed, not what is applied; ``planDigest`` covers them. Never on a managed streaming + plan, whose apply runs no OpenTofu and checks its actions against its artifacts one for + one.""" + actions = list(plan.get("actions") or []) + present = {a.get("action_id") for a in actions if isinstance(a, dict)} + added = [ + action + for action in _output_port_actions(contract, args, logger) + if action.get("action_id") not in present + ] + if not added: + return + for action in added: + actions.append({"step": len(actions) + 1, "depends_on": [], **action}) + plan["actions"] = actions + plan["total_actions"] = len(actions) + graph = plan.get("dependency_graph") + if isinstance(graph, dict) and isinstance(graph.get("nodes"), list): + graph["nodes"] = graph["nodes"] + [a.get("action_id") for a in added] + + +def _output_port_actions( + contract: Dict[str, Any], args, logger: logging.Logger +) -> List[Dict[str, Any]]: + """The hook's actions, from the contract as the apply reads it: its ``{{ env.* }}`` + resolved, the one provider ``_resolve_provider`` picks (``--provider``, or the contract's + only cloud: an ambiguous or mismatched one lists nothing, since the apply refuses it too), + and the contract's directory known, where a local dbt build's project is read from. + In-tree plugins only (``IAC_PLUGINS``: nothing is loaded from an entry point to plan). A + hook that fails lists nothing, with a warning: ``fluid validate`` and ``fluid apply`` + refuse the contract with the reason, and the plan of the rest stays readable.""" import copy # Function-local: ``iac`` pulls every provider plugin, off the ``fluid --help`` cold path. @@ -894,17 +916,17 @@ def _append_output_port_actions( resolved = resolve_env_templates_in_contract(copy.deepcopy(dict(contract))) except Exception as exc: # noqa: BLE001 - the plan of everything else still stands warn(logger, "plan_output_ports_skipped", error=f"{type(exc).__name__}: {exc}") - return + return [] try: provider = _resolve_provider(resolved, getattr(args, "provider", None) or "auto") except CLIError: - return + return [] hook = getattr(IAC_PLUGINS.get(provider), "planned_output_ports", None) if not callable(hook): - return + return [] try: with contract_source(source_contract_file(getattr(args, "contract", None))): - added = [dict(action) for action in hook(resolved)] + return [dict(action) for action in hook(resolved)] except Exception as exc: # noqa: BLE001 - the plan of everything else still stands warn( logger, @@ -913,19 +935,7 @@ def _append_output_port_actions( kind=getattr(exc, "kind", None), error=f"{type(exc).__name__}: {exc}", ) - return - actions = list(plan.get("actions") or []) - present = {a.get("action_id") for a in actions if isinstance(a, dict)} - added = [action for action in added if action.get("action_id") not in present] - if not added: - return - for action in added: - actions.append({"step": len(actions) + 1, "depends_on": [], **action}) - plan["actions"] = actions - plan["total_actions"] = len(actions) - graph = plan.get("dependency_graph") - if isinstance(graph, dict) and isinstance(graph.get("nodes"), list): - graph["nodes"] = graph["nodes"] + [a.get("action_id") for a in added] + return [] def _plan_schedule_actions( diff --git a/fluid_build/iac/providers/databricks_genie.py b/fluid_build/iac/providers/databricks_genie.py index 21afb6e4..16429da7 100644 --- a/fluid_build/iac/providers/databricks_genie.py +++ b/fluid_build/iac/providers/databricks_genie.py @@ -1034,36 +1034,45 @@ def _broadest(acl: Any, folder: str) -> Tuple[str, str]: level this module does not know could hide a grant to every user.""" entries = acl.get("access_control_list") if isinstance(acl, Mapping) else None if not isinstance(entries, list) or not entries: - raise GenieError( - f"the permissions on the folder {folder} came back without an access control list, " - "so whether every workspace user can edit or manage it is unknown.", - (_remediation(None, f"read the permissions on the folder {folder}"),), - ) + raise _unreadable(folder, "came back without an access control list") best = (ALL_USERS, "") for entry in entries: group = entry.get("group_name") if isinstance(entry, Mapping) else None if group not in ALL_USERS_GROUPS: continue - held = entry.get("all_permissions") - levels = [ - str(item.get("permission_level") or "").upper() - for item in (held if isinstance(held, list) else ()) - if isinstance(item, Mapping) - ] - unknown = [level for level in levels if level not in _LEVEL_RANK] - if not levels or unknown: - raise GenieError( - f"the permissions on the folder {folder} do not say plainly what the {group} " - f"group holds ({unknown or held!r}), so whether every workspace user can edit " - "or manage it is unknown.", - (f"Check the permissions on {folder}, then re-run fluid apply.",), - ) - for level in levels: + for level in _held_levels(entry, folder): if _LEVEL_RANK[level] > _LEVEL_RANK.get(best[1], 0): best = (str(group), level) return best +def _held_levels(entry: Mapping[str, Any], folder: str) -> List[str]: + """The levels a group of every user holds on ``folder``, upper-cased; an entry that does + not say them plainly stops the apply.""" + held = entry.get("all_permissions") + levels = [ + str(item.get("permission_level") or "").upper() + for item in (held if isinstance(held, list) else ()) + if isinstance(item, Mapping) + ] + unknown = [level for level in levels if level not in _LEVEL_RANK] + if not levels or unknown: + raise _unreadable( + folder, + f"do not say plainly what the {entry.get('group_name')} group holds " + f"({unknown or held!r})", + ) + return levels + + +def _unreadable(folder: str, what: str) -> GenieError: + return GenieError( + f"the permissions on the folder {folder} {what}, so whether every workspace user can " + "edit or manage it is unknown.", + (f"Check the permissions on {folder}, then re-run fluid apply.",), + ) + + def _folder_object_id(client: Any, folder: str) -> str: """The object id of the folder ``parentPath`` names, for the Permissions API path; a folder that does not exist, or a path that is no folder, stops the apply with what to do.""" From 8db4173af280944206ce3819ecf0108b5304e239 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Fri, 13 Nov 2026 00:11:07 +0100 Subject: [PATCH 7/7] fix(databricks): the Genie move's 409 advice and the plan hook's docstring are accurate A move into parentPath carries parent_path alone, so a 409 there is a clash with the title the space has now, not the contract's. The advice now says to rename either space, and that the apply sets the contract's title after the move. The plan hook's docstring said it loads no entry-point plugin, but _resolve_provider loads an operator-allowed one, as the apply does, and the docstring now says so. Both came from the re-check after the review round. --- fluid_build/cli/plan.py | 9 +++++---- fluid_build/iac/providers/databricks_genie.py | 8 ++++---- tests/iac/test_iac_databricks_genie.py | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/fluid_build/cli/plan.py b/fluid_build/cli/plan.py index d457befd..bb47be27 100644 --- a/fluid_build/cli/plan.py +++ b/fluid_build/cli/plan.py @@ -899,10 +899,11 @@ def _output_port_actions( """The hook's actions, from the contract as the apply reads it: its ``{{ env.* }}`` resolved, the one provider ``_resolve_provider`` picks (``--provider``, or the contract's only cloud: an ambiguous or mismatched one lists nothing, since the apply refuses it too), - and the contract's directory known, where a local dbt build's project is read from. - In-tree plugins only (``IAC_PLUGINS``: nothing is loaded from an entry point to plan). A - hook that fails lists nothing, with a warning: ``fluid validate`` and ``fluid apply`` - refuse the contract with the reason, and the plan of the rest stays readable.""" + and the contract's directory known, where a local dbt build's project is read from. The + plugin is the one the apply runs: ``_resolve_provider`` loads an operator-allowed + entry-point plugin as the apply does. A hook that fails lists nothing, with a warning: + ``fluid validate`` and ``fluid apply`` refuse the contract with the reason, and the plan of + the rest stays readable.""" import copy # Function-local: ``iac`` pulls every provider plugin, off the ``fluid --help`` cold path. diff --git a/fluid_build/iac/providers/databricks_genie.py b/fluid_build/iac/providers/databricks_genie.py index 16429da7..024f5960 100644 --- a/fluid_build/iac/providers/databricks_genie.py +++ b/fluid_build/iac/providers/databricks_genie.py @@ -1130,11 +1130,11 @@ def _move(client: Any, moving: Mapping[str, Any], space: Space, say: Say, dry_ru if _status(exc) == 409: raise GenieError( f"could not move space {space_id} into {space.parent_path}: another space there " - f"is already called {moving.get('title')!r}, and Genie keeps titles unique in a " - "folder (measured for a create and a rename).", + f"is already called {moving.get('title')!r}, the title this space has now, and " + "Genie keeps titles unique in a folder (measured for a create and a rename).", ( - f"Give this space a title of its own with extensions.{EXTENSION_KEY}.title, " - "or rename the other space, then re-run fluid apply.", + "Rename that space, or this one in Genie, then re-run fluid apply: it sets " + f"the contract's title (extensions.{EXTENSION_KEY}.title) after the move.", ), ) from exc raise GenieError( diff --git a/tests/iac/test_iac_databricks_genie.py b/tests/iac/test_iac_databricks_genie.py index b367ce6e..56b3040f 100644 --- a/tests/iac/test_iac_databricks_genie.py +++ b/tests/iac/test_iac_databricks_genie.py @@ -1452,7 +1452,7 @@ def test_a_move_onto_a_title_the_folder_holds_says_so(fake): with pytest.raises(genie.GenieError) as exc: Run(fake)(site_contract(parentPath=SALES)) assert "another space there is already called 'Site 360'" in str(exc.value) - assert "title of its own" in exc.value.remediation[0] + assert "Rename that space, or this one in Genie" in exc.value.remediation[0] def test_folder_permissions_this_identity_cannot_read_stop_the_apply(fake):