[mateoHernandez123]

Description

• Bug fix
• New feature

Second of four stacked PRs for Unity Catalog (CXH-2721). A catalog is the named container a customer picks in an access review. Its parent is the metastore. The workspace host is only the access path.

#70 is merged, so this PR now targets staging directly.

What the connector syncs

Before, on staging. The metastore is a leaf.

flowchart TD
  account[account]
  user[user]
  group[group]
  sp[service principal]
  role[role]
  workspace[workspace]
  metastore[metastore]
  account --> user
  account --> group
  account --> sp
  account --> role
  account --> workspace
  account --> metastore

Loading

After this PR. Catalogs hang from the metastore, with their own entitlements and grants.

flowchart TD
  account[account]
  user[user]
  group[group]
  sp[service principal]
  role[role]
  workspace[workspace]
  metastore[metastore]
  catalog[catalog]
  account --> user
  account --> group
  account --> sp
  account --> role
  account --> workspace
  account --> metastore
  metastore --> catalog
  style catalog fill:#1f6feb,color:#fff,stroke:#1f6feb

Loading

Order	PR	Delivers	Base	State
1	#70	Client, routing, metastore	staging	Merged
2	#71	Catalog	staging	This PR
3	#72	Schema	#71	Open
4	#73	Table and volume	#72	Open

Sync:

• Catalogs (catalog, NEW, opt-in) — child of metastore, keyed {metastore_id}::{name}. One entitlement per catalog privilege, plus a read-only owner.
• The connector reads catalogs through every running in-scope workspace attached to the metastore. An ISOLATED catalog is listed only by the workspaces bound to it, so each workspace is read. A catalog that several workspaces list is emitted once per route. The store keys a resource by (external_id, sync_id) and upserts, so the copies collapse into one row. Measured on a live tenant: with every route pushed twice, a three-catalog metastore still synced three catalogs.
• A catalog whose privilege list cannot be proved complete fails List with PermissionDenied. The error names the catalog and the remedy. ConductorOne does not remove the catalog. See Auth below.
• A permissions 404 skips only that catalog when the body carries CATALOG_DOES_NOT_EXIST. Any other 404 is FailedPrecondition, so a refusal never reads as an empty page.
• system and samples are left out only while their privilege list cannot be proved complete. The service principal that owns the metastore reads them in full, and then both are synced.
• A metastore the account stopped listing has no catalogs to list. The catalog List reports none instead of failing, because the metastore syncer removes the parent in the same sync.
• List is what refuses a partial privilege list. Grants does not prove it a second time.
• Metastores and the existing types are unchanged.

Provisioning:

• Grant/Revoke catalog privileges — PATCH .../permissions/catalog/{name}. A pre-read past the cache decides GrantAlreadyExists and GrantAlreadyRevoked. The echoed assignments confirm the change.
• Owner is read-only. A Hive-era alias or a metastore privilege is rejected with InvalidArgument.
• Grant and Revoke rebuild the route before they write. The workspace on the profile is reused only while it is still in sync scope and still attached to the metastore. The resource is rebuilt from the live read, so trust is decided from the owners Databricks returns now, not from the owners the last sync stored.

A catalog that no workspace lists is not proof of deletion. An unbound workspace answers the same 404 as a deleted catalog. The connector asks the metastore owner for the listing with include_unbound=true before it answers:

flowchart TD
  start[Grant or Revoke] --> route[rebuild the route]
  route -->|a workspace serves the catalog| write[PATCH permissions]
  route -->|every workspace answers CATALOG_DOES_NOT_EXIST| proof{connector owns the metastore?}
  proof -->|no| unknown[FailedPrecondition: deletion cannot be proved]
  proof -->|yes| unbound[list with include_unbound=true]
  unbound -->|catalog is in the listing| exists[FailedPrecondition: it exists, out of reach]
  unbound -->|catalog is absent| gone[Revoke: GrantAlreadyRevoked / Grant: NotFound]

Loading

A deleted metastore takes the same two answers: Revoke returns GrantAlreadyRevoked, Grant returns NotFound.

Measured on the Baton account, where the service principal owns the metastore:

• Revoke on a catalog that does not exist returns GrantAlreadyRevoked with no error.
• Grant on the same catalog returns NotFound.
• A Grant and Revoke round trip on a real catalog writes and reads back correctly.
• GET /catalogs/{name} and GET /permissions/catalog/{name} both answer 404 CATALOG_DOES_NOT_EXIST for a catalog that does not exist.

Auth:

Credentials are unchanged. There is no new config field.

A catalog is synced when the service principal owns the metastore, owns the catalog, holds MANAGE on the catalog, or is a direct Admin of the workspace whose admins group owns it. The admins group is named _workspace_admins_<workspace>_<id>, and the connector reads the workspace id off that name. The check runs against that workspace whatever route the catalog is listed through, because permission visibility follows the caller, not the host.

Databricks answers anyone else with a 200 and only their own grants. A catalog that meets none of these rules fails List with PermissionDenied naming the remedy. ConductorOne does not remove it. Grant and Revoke on such a catalog fail with PermissionDenied and send no PATCH.

Architecture highlights:

• Shared securable path: securableLevel holds what differs per level — the securable type, the privilege set, the remedies, the name parser, the gone error codes and the trust rule. syncSecurableGrants, grantSecurable and revokeSecurable are shared by metastore and catalog.
• Profile: List writes the workspace, workspace id, workspace name, catalog owner and metastore owner. Grants reads the workspace back, which saves an account round trip per catalog.
• Routing: pushRoute resolves the usable workspaces once per metastore and stores one page state per workspace in the pagination.Bag. Later pages of the same workspace do not resolve the route again.
• Privileges: the catalog set follows the Databricks privileges reference. The PATCH accepts and echoes every entry on a live tenant, including the ones the databricks-sdk-go Privilege enum omits.
• include_unbound answers whether a catalog exists. It does not grant access to it, so List does not use it. A workspace cannot read the grants of a catalog it is not bound to.

Useful links:

• Linear ticket CXH-2721 — catalog sync
• Linear ticket CXE-1551 — SDK: redundant writes for resources returned through overlapping routes
• List catalogs
• Limit catalog access to specific workspaces
• Unity Catalog privileges
• Update permissions on a securable

[linear-code]

CXH-2721

[FeliLucero1]

Did a pass on this one. Main thing is the first comment: Revoke can say "already revoked" while the privilege still exists. The rest are perf and dupe stuff.

Comments tagged (out of scope / non-blocking) are low-confidence or nits I didn't fully verify, so treat them as FYI. Also out of scope: the databricks-connector: error prefix vs baton-databricks: is a convention that predates this PR, not something it introduced.

Heads up: the unreachable-catalog thing below is still the same in #72 / #73.

[FeliLucero1]

[sergiocorral-conductorone]

Automated review (follow-up/first pass) — drafted 3 → confirmed 3, low-confidence 0, dropped 0, added 0 (panel: opus-5.5 / gpt-6-astra / gemini-4-argon, all high).

[FeliLucero1]

Pass on 95d6df31. CI is green. The every-workspace walk, removing the dedupe (the SDK upserts on (external_id, sync_id), checked) and the first-page-only trust check all look good. I put the cost numbers and the docs follow-up as replies in the existing threads. These three are new.

(out of scope) Provisioning walks GetCatalog across workspaces one by one until one serves the catalog, so up to W calls per Grant/Revoke. Trying the profile's workspace first would make the common case a single call.

[luisina-santos]

This is a lot of work to list a metastore's catalogs. Before the first page, pushRoute calls ListWorkspaces, ListMetastoreWorkspaces and ListMetastores for every metastore, repeating what the metastore List just did in the same sync. Then it pushes a JSON catalogRoute into the bag's ResourceID, which is supposed to hold a resource id, and unmarshals it back on each page.

The SDK already gives List what it needs: the metastore id as parentResourceID, and a page token scoped to that parent. Can the metastore List write metastoreID → workspace (and the owner) to the session store (attr.Session), and the catalog List read it from there? Then the page token is just the Databricks one, with no bag, no route building and no JSON in ResourceID. The connector would need the session store enabled.

If catalogs really have to be read through every attached workspace, not just one, please link the Databricks docs or a test against the tenant that shows a catalog missing from another workspace's list.

[mateoHernandez123]

Not adding a session store. A catalog can be ISOLATED to one workspace, so List has to ask every running workspace attached to the metastore. On the Baton account test_workspace is ISOLATED; storing one route and skipping the others would drop it. The page token still carries the route so later pages of that workspace do not rebuild it.

[luisina-santos]

Reopening. The ISOLATED point is fair, but it argues against storing one workspace, and that's not what I'm asking for. The session entry can hold every running attached workspace plus the owner. The repeated ListWorkspaces / ListMetastoreWorkspaces / ListMetastores per metastore and the JSON in ResourceID are still there (catalog.go:317-363, :232-233). The full reasoning is in the review body.

[sergiocorral-conductorone]

Automated review.

(drafted 4 → confirmed 4, low-confidence 0, dropped 0, added 2; panel: opus-5.5 / gpt-6-astra / gemini-4-argon, all high)

[agustin-conductor]

there is an " include_unbound" param that might be worth exploring in order to avoid this per workspace multiple loop. Also, as an alternative, we could just define Catalog as child resource when creating the Workspace resources, that way we would get the ws as parent in the call to this List(). Doing so I believe it does not create the parent-child relationship, that is defined by setting the parentId when creating the resource (in this case we would create the catalog with the metastoreID as parent). Also, I think we can avoid the logic to dedup resources as long as we emit the same, the sdk will take care of deduplication

[mateoHernandez123]

Thanks Agus, I measured both.

include_unbound is real and the client now sends it, but I would not use it to replace the per-workspace walk. The reference says it is "effective only if the user has permission to update the catalog-workspace binding", and the binding page is explicit that "access from an unbound workspace is denied, even for users with explicit privilege grants on the catalog", while "metastore admins and catalog owners can see all catalogs in the metastore, regardless of binding".

So the flag answers "does this catalog exist?", not "can I read its grants through this workspace?". If List used it, we would emit catalogs whose /permissions read 404s from that route. That is zero grants, and C1 deletes the grants it already had. That is why the flag is only in the deletion proof for Grant and Revoke, where the question really is a listing one.

What I could measure on the Baton account: the parameter is accepted, no 400, and it returns the same three catalogs. The account has one workspace, so nothing there is unbound. I could not build a genuinely unbound catalog either, because the API refuses to create a catalog on that metastore (Default Storage needs the UI). So the 404-on-permissions half comes from the docs, not from a probe. If you know a tenant with two workspaces I would like to close that one properly.

On the dedupe: already removed in 95d6df3. I measured it with every route pushed twice, and a three-catalog metastore still synced three catalogs, so the store upserts on (external_id, sync_id) exactly as you said. I opened CXE-1551 on the SDK for the redundant writes themselves.

The catalog-as-child-of-workspace idea I have not tried. My hesitation is that the hierarchy C1 should show is metastore to catalog, so the List parent and the resource parent would disagree. Happy to pick it up as a follow-up if you think that is fine.

[mateoHernandez123]

Context for a future PR: the repeated catalog writes are an SDK ticket now

Leaving this here so whoever picks it up later does not have to reconstruct it from the review threads.

Why this connector emits the same catalog more than once. A catalog belongs to a metastore, but every Unity Catalog read is addressed to a workspace host. A catalog with isolation_mode = ISOLATED is listed only by the workspaces it is bound to, so List reads the catalogs through every running in-scope workspace attached to the metastore. A catalog that several of those workspaces list is therefore emitted once per route.

Why the connector no longer deduplicates them. It used to keep a set of the catalogs it had already emitted. That was removed in 95d6df3, because the set only holds for one sync and carrying it across syncs made the second sync drop catalogs it had emitted in the first. The store already keys a resource by (external_id, sync_id) and upserts on conflict, so the copies collapse into one row on their own. Measured on the Baton account: with every route pushed twice, a three-catalog metastore still synced three catalogs.

So the result is correct. What is left is the cost: the SDK still processes and submits every repeated resource to its writer, rather than collapsing them before the write.

That part is now CXE-1551 on the Connector Engine team. It asks the SDK to make the semantics of overlapping enumeration routes explicit and to avoid the redundant writes where that is safe — preserving the latest data when repeated ids carry different payloads, and keeping memory bounded on large paginated syncs. The ticket also lists two other connectors with the same shape: baton-azure re-emits inherited role assignments while walking descendant scopes, and baton-boomi emits a user per role assignment.

What that means for this repo. Nothing has to change here to merge. If the SDK lands a deduplication contract, the thing to revisit is the comment on pushRoute in pkg/connector/catalog.go, which currently documents the measurement above as the reason no connector-side set is needed. A future PR should point it at whatever the SDK guarantees instead, and re-run the same measurement to confirm the count is still right.

Please do not reintroduce a connector-side dedupe set as a workaround in the meantime. It is the thing that caused the dropped-catalog bug.

[ggreer]

I didn't notice anything incorrect, but I'm not sure the code will be fast enough for larger accounts. It really depends on how many catalog & metastore resources there are.

[ggreer]

Is it cleaner to make revokeSecurable() take the principal & the entitlement instead of cloning the proto and setting a new entitlement? And metastoreLevel is always the same, so it could be omitted as well. Also considering how many metastoreBuilder arguments passed to syncSecurableGrants and revokeSecurable, does it make sense to make these functions methods on metastoreBuilder?

Then calls would look something like: m.revokeSecurable(ctx, principal, routed) and m.grantSecurable(ctx, principal, routed)

[ggreer]

It seems a little cleaner to iterate over the values instead of looking them up by index:

	for _, workspace := range usable {
		admin, err := isWorkspaceAdmin(ctx, client, ownPrincipal, strconv.Itoa(workspace.ID), annos)
		if err != nil {
			return nil, err
		}
		if admin {
			return &workspace, nil
		}
	}

[ggreer]

How many metastores are there? pickReadableWorkspace() is called in a loop, which calls isWorkspaceAdmin(), which calls client.ListWorkspaceMembers(). If there are many metastores, that could cause us to do a lot of API calls in each List() call.

[ggreer]

syncSecurableGrants gets called on every catalog resource, and it calls ensureGrantsAreTrustworthy(), which calls catalogGrantsAreTrustworthy(), which calls isWorkspaceAdmin(), which calls client.ListWorkspaceMembers(). So for every catalog resource, we're listing workspace members. Also catalogGrantsAreTrustworthy() calls holdsCatalogManage(), which lists all permissions to find whether the manage permission is present.

I think this is going to result in a lot of API calls. Is there any other way to get this info?

[FeliLucero1]

Pass on 3cabca96. CI is green, and 8bf69687 resolves the Grants re-probe nicely. I put the stale metastore owner and the CATALOG_DOES_NOT_EXIST premise as replies on Sergio's threads. Two new ones below.

Non-blocking:

• Stale comments: catalog.go:585 says Revoke doesn't return GrantAlreadyRevoked for an unlisted catalog (it does now, via confirmCatalogGone), and :601-603 says the profile workspace "is kept" (the resource is always rebuilt now). connector.mdx:198 still says system/samples were "never synced". README and docs-info still don't cover catalogs.
• Worth a line in the docs: Revoke on a deleted catalog is a permanent FailedPrecondition unless the SP owns the metastore.
• Catalog Grant/Revoke now makes 4 calls before the write (ListMetastores + ListWorkspaces + ListMetastoreWorkspaces + GetCatalog), and the fallback probes the profile workspace twice. The "live" owner reads also go through the cached GET (request.go:148-152, a 200 is kept up to 1h), so the owner might not be that live in a long-lived process.

(out of scope) #72's schemaBuilder.pushRoute returns errCatalogGone / errMetastoreGone raw too, so it needs the same handling.

[FeliLucero1]

[medium] attached now maps METASTORE_DOES_NOT_EXIST to errMetastoreGone, and assignmentListError passes it through as-is. But pushRoute only treats the sentinel as "no catalogs" when it comes from metastoreOwner (:341), not from here. So a metastore deleted between metastore List and catalog List fails the sync with a bare errors.New: no prefix, no gRPC code, so C1 sees Unknown and retries. TestCatalogListIsEmptyWhenTheMetastoreIsGone only drops it from the account listing, so it never hits this path. metastore.go:124-126 returns the raw sentinel the same way (failing there is intentional, but the message and code got lost).

[FeliLucero1]

[low] Any error on the include_unbound listing (429, 5xx, timeout) ends up as FailedPrecondition ... %v. C1 treats that as fatal on the first attempt, so a transient blip while proving the deletion kills the revoke permanently. It also drops the %w. Maybe only map 401/403/404 to FailedPrecondition (like refuseUnreadable) and let everything else pass through wrapped?

[sergiocorral-conductorone]

Automated review (follow-up round on new commits).

Stats: drafted 2 → confirmed 2, low-confidence 0, dropped 0, added 0 (panel: opus-5.5 / gpt-6-astra / gemini-4-argon, all high).

Findings not anchorable to a diff line:

• pkg/connector/catalog.go:325 — [minor] The new errMetastoreGone branch in pushRoute (:341) is reachable only in the fake. scope.attached runs first, at :323. For a deleted metastore, Databricks answers the assignment list with 404 METASTORE_DOES_NOT_EXIST, which attached now maps to errMetastoreGone. assignmentListError then passes it through unwrapped, so List returns a bare errors.New (gRPC Unknown) instead of the empty result the PR description promises ("The catalog List reports none instead of failing"). TestCatalogListIsEmptyWhenTheMetastoreIsGone only drops the metastore from ListMetastores while the fake still serves its workspace list, so it doesn't hit this order. The same unwrapped sentinel now leaks from metastore List (metastore.go, assignmentListError in the loop) when a metastore is deleted between ListMetastores and its assignment read. Before this commit that path returned FailedPrecondition. Could pushRoute check errors.Is(err, errMetastoreGone) after attached too, could the sync paths never return the bare sentinel, and could the test make the fake's assignment list 404 with METASTORE_DOES_NOT_EXIST?

[sergiocorral-conductorone]

[major] The include_unbound listing decides whether Revoke answers GrantAlreadyRevoked, but ListCatalogs reads through the cached c.Get (unity_catalog.go:106). request.go says the cache holds a GET 200 for an hour and that a read deciding a write must use GetUncached. On the earlier GetCatalog thread you said the deletion read would go through GetUncached. That was true in b41db8b but isn't at this head.

How it goes wrong: an earlier Revoke caches the unbound listing for a workspace. A catalog created after that read is synced, then unbound from every in-scope workspace. A Revoke within the hour gets the cached listing without it and returns GrantAlreadyRevoked, so C1 records the revoke as done while the privilege is still held in Databricks. The other direction: a catalog deleted after a cached listing that still includes it keeps failing Revoke with FailedPrecondition ("still exists") for up to an hour.

The owner fix has the same gap. metastoreOwner (ListMetastores) and catalogThrough (GetCatalog) also go through c.Get. Within an hour of a sync in the same process, the "live" owner is the cached one, i.e. the same value the profile carried. Could ListCatalogs take a no-cache option for the include_unbound call (or use databricks.WithoutCache(ctx) in confirmCatalogGone), and could the provisioning-path ListMetastores/GetCatalog reads bypass the cache too? The same applies to the metastore profile shortcut, which still decides trust from the profile owner.

[luisina-santos]

Follow-up on the replies to my Oct 28 threads. I've reopened the ones I don't think are addressed yet and replied in each. The main one is the route building in catalog List, so here's the full reasoning for it.

Why I'd like catalog List to reuse what metastore List already read

1. It reads the same data twice in one sync. Metastore List already calls ListMetastores, ListWorkspaces and ListMetastoreWorkspaces per metastore (metastore.go:105-124). The catalog List runs right after it for the same metastore, and pushRoute calls all three again (catalog.go:317-363). metastoreOwner reads the whole account metastore list just to find one owner, so with M metastores the account list is read M+1 times instead of once.
2. These are account-level calls, and the extra count grows with the account. Every metastore adds three more account-level calls, which share the account's rate limits with the rest of the sync. Greg's review raises the same concern about larger accounts.
3. Two reads in one sync can disagree. If a workspace stops or is detached between the metastore List and the catalog List, the metastore's profile names one route and its catalogs are read through a different set. Reading it once avoids that.
4. The page token shouldn't carry this. PageState.ResourceID is meant to hold a resource id. Here it holds a JSON catalogRoute (catalog.go:232-233, 353-362) that is unmarshaled on every page, and every token carries the metastore owner.
5. ISOLATED catalogs don't rule it out. I agree catalogs have to be read through every running attached workspace. The session entry can hold that whole list, so nothing limits it to one workspace.
6. The HTTP cache isn't the answer. Working around it is what the README cache-size section was for, and we removed that section for this reason.

Suggestion: in metastore List, write metastoreID -> {running in-scope workspaces, owner} to attr.Session (SyncOpAttrs.Session is already in the pinned baton-sdk v0.42.5, but the connector would need the session store enabled). Catalog List reads that entry on its first page and walks the workspaces, with the bag holding only the workspace deployment name and the Databricks page token. Keep today's live resolution as the fallback when the entry is missing. For Grant/Revoke the same idea applies through the profile (see the catalog.go:569 thread).

[luisina-santos]

Reopening. The ISOLATED point is fair, but it argues against storing one workspace, and that's not what I'm asking for. The session entry can hold every running attached workspace plus the owner. The repeated ListWorkspaces / ListMetastoreWorkspaces / ListMetastores per metastore and the JSON in ResourceID are still there (catalog.go:317-363, :232-233). The full reasoning is in the review body.

[luisina-santos]

Reopening. These catalogs are new in this PR, so always leaving them out doesn't delete anything that's in C1 today; they just never show up. What I want to avoid is exactly what the comment at catalog.go:268-272 says: they show up while the connector owns the metastore, and get deleted (grants included) when it stops owning it. Please pick one behaviour that doesn't depend on ownership. Always leaving them out seems simplest, since without metastore ownership Databricks doesn't let the connector read their full privilege list.

[luisina-santos]

Follow-up to #71 (comment). The routing change looks good. What's still missing are the tenant runs I asked for: a Grant and a Revoke from C1, plus one where the profile workspace is gone, to show the fallback works via CLI.

[luisina-santos]

Follow-up to #71 (comment). The builder isn't rebuilt per page; only the page token is, so a map on the builder would still be filled on the next page. Since the builder can also outlive one sync, the session store (per sync) is a better fit than a map on the builder: key it by workspace id and isWorkspaceAdmin runs once per workspace. The service principal owning the metastore on our test account doesn't tell us much about customer accounts where the workspace admins group owns the catalogs.

[luisina-santos]

Follow-up to #71 (comment). The profile workspace is only checked here, after metastoreOwner (ListMetastores), scopedWorkspaces (ListWorkspaces) and attached (ListMetastoreWorkspaces) have already run (:620-627). So the profile saves the GetCatalog walk, but not the account-level calls. Can the profile path go first: one GetCatalog through the profile workspace, with live resolution only when that doesn't return the catalog? If the owner has to be fresh, one ListMetastores is fine; the workspace listings are what the profile should save. resolveChange also still runs the access check again before the PATCH (securable_helpers.go:278).

I also asked for runs against the tenant: a Grant and a Revoke from C1, plus one where the profile workspace is gone. The reply only shows a GET /catalogs/test_workspace. Can you share those runs?

[luisina-santos]

Follow-up to #71 (comment). After 3cabca96 this comment is out of date: Revoke now returns GrantAlreadyRevoked (:588-589) once confirmCatalogGone proves the catalog is deleted. Please update it or remove it.