[mateoHernandez123]

Description

• Bug fix
• New feature

Third of four stacked PRs for Unity Catalog (CXH-2722). A schema sits between a catalog and the tables and volumes an access review certifies. A principal can hold USE_SCHEMA without holding anything on those tables, so the schema is its own resource.

#70 is merged. #71 merges before this one.

What the connector syncs

Before, with #71 applied. The catalog is a leaf.

flowchart TD
  account[account]
  metastore[metastore]
  catalog[catalog]
  other["user, group, service principal, role, workspace"]
  account --> other
  account --> metastore
  metastore --> catalog

Loading

After this PR. Schemas hang from the catalog, with their own entitlements and grants.

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

Loading

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

Sync:

• Schemas (schema, NEW) — child of catalog, read with GET /api/2.1/unity-catalog/schemas?catalog_name=. Keyed {metastore_id}::{catalog}.{schema}. The privilege set is the catalog set without BROWSE, CREATE_SCHEMA and USE_CATALOG, plus a read-only owner.
• A 404 on the schema listing is an error, not an empty catalog. An empty page would read as deletions in ConductorOne.
• information_schema is skipped. Its system views are owned by principals that do not exist in SCIM.
• A catalog outside the sync scope takes its schemas with it. That rule comes from [CXH-2721] - Add Unity Catalog catalog sync - Databricks Connector #71 and is unchanged here.
• The types above are unchanged from [CXH-2720] - Add Unity Catalog client, access-path routing, and metastore sync - Databricks Connector #70 and [CXH-2721] - Add Unity Catalog catalog sync - Databricks Connector #71.

flowchart LR
  catalog[catalog]
  info[information_schema]
  schema[schema]
  catalog -.->|skipped| info
  catalog --> schema
  schema --> entitlements[one entitlement per schema privilege]
  schema --> owner[read-only owner]

Loading

Provisioning:

• Grant/Revoke schema privileges — the same PATCH .../permissions path and the same echo-confirmed idempotency as the catalog: GrantAlreadyExists and GrantAlreadyRevoked.
• A revoke that fails with SCHEMA_DOES_NOT_EXIST or CATALOG_DOES_NOT_EXIST is already revoked. Only the error code proves the absence, because an unbound or unprivileged workspace answers 404 as well.
• Owner is read-only.

Auth:

Credentials are unchanged. There is no new config field.

Schema List fails with PermissionDenied unless the service principal owns the metastore, owns the catalog, or holds MANAGE, USE_SCHEMA or ALL_PRIVILEGES on the catalog. Workspace Admin alone is not enough.

A direct Admin is also enough when the catalog is owned by that workspace's admins group. The connector reads the workspace id off the group name _workspace_admins_<workspace>_<id> and checks that workspace, whatever route the schema is listed through. Permission visibility follows the caller, not the host.

The visibility probe walks the attached workspaces. A 401 or 403 leaves the privilege unproven and the probe tries the next workspace. Any other probe error fails that catalog's schema List. The page token carries the workspace and the metastore owner. It does not carry the catalog owner.

Architecture highlights:

• Shared trust probe: holdsPrivilegeOn is the one uncached read of the connector's own effective privileges. Each level passes the privileges it accepts and the error codes that prove the securable is gone. The schema level names none, so every 404 there leaves the privilege unproven rather than claiming an absence.
• Principals: schema Grants uses the same principal path as [CXH-2720] - Add Unity Catalog client, access-path routing, and metastore sync - Databricks Connector #70. It reads the full account SCIM only when assignments exist, and uses the Find* filters for an owner-only grant.

Useful links:

• Linear ticket CXH-2722 — schema sync
• List schemas
• Unity Catalog privileges
• Update permissions on a securable

[linear-code]

CXH-2722

[FeliLucero1]

Did a pass on this one. The main things are the two mediums: Revoke on a dropped schema fails instead of returning already-revoked, and List swallows a 404 into an empty page. The rest is perf/docs/nits.

Stuff tagged (out of scope) is inherited from #71 or is stack-wide, so FYI, not blocking for this PR. Also checked the schema privilege set against the Databricks docs and it matches exactly (27, minus BROWSE / CREATE_SCHEMA / USE_CATALOG) 👍

[FeliLucero1]

[FeliLucero1]

Another pass. Nice work on the live checks (SCHEMA_DOES_NOT_EXIST, the privilege PATCH) and moving Grant/Revoke onto the shared helpers 👍

The main things this round are about schemas silently disappearing: the partial-listing skip (replied on the :73 thread), the usable[0] regression (replied on Sergio's :353 thread), and a 404 that still comes out as NotFound. The rest are nits. (out of scope): the same NotFound-wrapping shape exists in catalog.go pushRoute (#71).

[sergiocorral-conductorone]

Automated review (follow-up)

Head moved since our last LGTM (ce73192 → abee6af). Resolved at head: the usable[0] regression (workspace walk is back, TestSchemaListTriesTheNextWorkspace restored), the partial-listing fail-closed behaviour, the docs nit on the workspace-admins-group exception. Still open from earlier rounds: FeliLucero1's thread on schema.go:269 (plain %w on the metastore-assignment / workspace / metastore-owner reads lets a NotFound become an SDK warning). New this round: 1 regression (Revoke on a dropped schema), 1 medium (built-in catalog skip), 2 low.

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

[ggreer]

Nitpick: Similar to the comment I made on #73, I don't think this line is needed. Child resources always work this way.

[ggreer]

I think this is an internal error. Not that it really matters, as InvalidArgument will fail the sync too.

[ggreer]

Should this be a log level higher than debug? I don't know how important it is or how likely it is to happen.

[ggreer]

baton-sdk will always error if a List(), Entitlements(), or Grants() returns the same page token as the previous call. So it's fine to delete this code.

https://github.com/ConductorOne/baton-sdk/blob/3bed541a13e01fbadd3ba94f0b58bb4ec369ea0c/pkg/connectorbuilder/resource_syncer.go#L177

[ggreer]

I think you could remove the workspace := usable[i] with:

Suggested change

	for i := range usable {
	for _, workspace := range usable {

[ggreer]

route.Workspace is really workspace.DeploymentName and route.WorkspaceID is strconv.Itoa(workspace.ID). So you could declare the schemaRoute in the json.Marshal() call if you changed these calls to use the values. Something like workspaceID := strconv.Itoa(workspace.ID) would avoid two int->string conversions, which doesn't matter for performance but might be a little more readable.

[ggreer]

Do we want to convert all of these into FailedPrecondition? Unauthorized and Forbidden will still cause baton-sdk to fail the sync, so there's no change in behavior there. But changing NotFound to FailedPrecondition means that a sync will fail instead of skipping the resource in some cases.

[FeliLucero1]

FWIW I think FailedPrecondition is what we want for these. With baton-sdk v0.40.0, a NotFound from List gets skipped as a warning, and nothing in the c1z says it was partial. On the c1 side, uplift deletes anything that's missing from the c1z (Bucket in pkg/mapper/resource/v2/uplift.go ~1259-1300 → DeleteAppResource). It doesn't check whether the parent was fully listed, and there's no mass-deletion threshold. So "skip" here ends up as "delete every schema + grant under that catalog" on a transient 404. Mateo reproduced it with a proxy injecting the 404: the old binary exited 0 with zero metastores. Failing the sync keeps the previous data. Happy to be corrected if the SDK handles this somewhere I missed though.

[FeliLucero1]

[low] since the #71 merge, catalogThrough returns errCatalogGone (plain errors.New, so codes.Unknown, no prefix), but this loop only special-cases deniesTrust, so it goes straight up through return err. If a catalog gets dropped between catalog List and schema List, the whole sync fails. catalog.go:266 already does if errors.Is(err, errCatalogGone) { continue }, so maybe return nil here for the same case?

Related: schemaListingIsComplete / schemaGrantsAreTrustworthy still pass nil goneCodes to holdsPrivilegeOn. So a schema dropped between List and Grants, when the SP only has MANAGE on the schema, ends in a misleading "grant MANAGE" PermissionDenied. That's the fix Sergio got at catalog level on #71, just not carried down here. Passing schemaLevel.goneErrorCodes would cover it.

[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).

[sergiocorral-conductorone]

[major] The walk here no longer gets past a workspace that answers 404 CATALOG_DOES_NOT_EXIST. After the #71 change picked up in this range, catalogThrough returns the plain errCatalogGone sentinel for that answer (catalog.go case databricks.IsSecurableGone(err, errorCodeCatalogGone): return nil, errCatalogGone). It used to return nil, nil, which this loop skipped with catalog == nil -> continue. errCatalogGone is not an API error, so deniesTrust is false, and the loop hits return err. The schema List then fails with gRPC code Unknown and the message "catalog no longer exists", and wsb is never tried.

The rest of this range treats a per-workspace CATALOG_DOES_NOT_EXIST as "this workspace doesn't serve it":

• workspaceForCatalog does if errors.Is(err, errCatalogGone) { continue }.
• It only reports gone when every workspace says so.
• Even then, confirmCatalogGone re-checks with include_unbound=true and can answer "still exists but no workspace in sync scope can list it".

So by the code's own model, an ISOLATED catalog bound only to usable[1] can come back gone from usable[0]. Through this List, that now breaks the sync for that catalog, which is the case 4279653524 and TestSchemaListTriesTheNextWorkspace exist for. The existing test misses it because serveCatalog sends a 404 with no error_code, which takes the deniesTrust branch. Suggest:

if errors.Is(err, errCatalogGone) || deniesTrust(err) {
	lastDeny = err // or track gone separately
	continue
}

Also add a variant of TestSchemaListTriesTheNextWorkspace where wsa's 404 carries error_code: CATALOG_DOES_NOT_EXIST. If every workspace says gone, the existing !sawCatalog FailedPrecondition still keeps C1's schemas, so that path stays fail-closed.

[sergiocorral-conductorone]

[minor] Follow-up on 4396462360: the goneErrorCodes fix only works when the trust check short-circuits on ownership. revokeSecurable runs resolveChange first, and resolveChange runs ensureGrantsAreTrustworthy before the permissions read that maps the gone codes. Here the probes pass nil goneCodes. In this range holdsPrivilegeOn then treats a 404 as "unproven" (deniesTrust includes 404), not as gone.

So for a principal that doesn't own the metastore or the catalog:

• Parent catalog dropped: both MANAGE probes (schema, then catalog) 404 with CATALOG_DOES_NOT_EXIST. Trust stays unproven, and Revoke returns PermissionDenied ("cannot read every privilege on schema…") instead of GrantAlreadyRevoked.
• Schema dropped, principal holds MANAGE only on the schema: same result.

The docs' MANAGE-per-catalog option is exactly this kind of principal, and the new test only covers a principal that owns the metastore (testSchema(..., testOwnPrincipal, ...)). Catalog level already does this (catalogGrantsAreTrustworthy passes []string{errorCodeCatalogGone} and maps errSecurableGone). Passing schemaLevel.goneErrorCodes to these two probes, and returning a gone result that revokeSecurable turns into GrantAlreadyRevoked, would make the fix hold for non-owners. You can test it with the existing fake by setting metastoreOwner/catalogOwner to someone else and putting the permErrorCode 404 on the probe reads.