Skip to content

direct: Do not drop MANAGE and other privileges not implied by ALL_PRIVILEGES - #6733

Open
denik wants to merge 3 commits into
mainfrom
denik/grants-17
Open

denik wants to merge 3 commits into
mainfrom
denik/grants-17

Conversation

@denik

@denik denik commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Changes

Keep the four privileges Unity Catalog does not include in ALL_PRIVILEGES (MANAGE, READ_METADATA, EXTERNAL_USE_SCHEMA, EXTERNAL_USE_LOCATION) when normalizing grants, and revoke a dropped one by name.

Why

On the direct engine, granting e.g. [MANAGE, ALL_PRIVILEGES] silently sent only ALL_PRIVILEGES. normalizeAssignments (added in #6064 for #6030) collapsed any principal holding ALL_PRIVILEGES down to just ALL_PRIVILEGES on both the config and the read side, so MANAGE was dropped from the request and the missing grant went unnoticed in the plan. Terraform sends both, which is why the bug showed up only after the engine default changed. ALL_PRIVILEGES does not imply these four: https://docs.databricks.com/aws/en/data-governance/unity-catalog/manage-privileges/privileges

Tests

New acceptance tests grants/schemas/all_privileges_manage (grant MANAGE + ALL_PRIVILEGES) and grants/schemas/manage_out_of_band (an out-of-band MANAGE grant is reconciled without a perpetual update), plus unit coverage. Local + cloud, both engines.

This pull request and its description were written by Isaac.

denik and others added 3 commits September 17, 2026 15:23
…rants

ALL_PRIVILEGES does not imply MANAGE, READ_METADATA, EXTERNAL_USE_SCHEMA or
EXTERNAL_USE_LOCATION. normalizeAssignments collapsed a principal holding
ALL_PRIVILEGES down to just ALL_PRIVILEGES, so granting e.g.
[MANAGE, ALL_PRIVILEGES] silently sent only ALL_PRIVILEGES and dropped MANAGE.
Keep the four excluded privileges through the collapse, and revoke a dropped
excluded privilege by name in buildGrantChanges (the ALL_PRIVILEGES "wipe" is
skipped when ALL_PRIVILEGES stays granted, so it can't clear them otherwise).

Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Both engines now send the same PATCH and read back the same grants, so the
per-engine out.requests/out.grants files are folded into output.txt. Extend the
normalizeAssignments unit test to all four privileges ALL_PRIVILEGES excludes.

Co-authored-by: Isaac <no-reply@databricks.com>
@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 3af49a3

Run: 35228076696

Env 🔄​flaky ✅​pass 🙈​skip Time
✅​ aws linux 284 15 5:19
✅​ aws windows 286 13 5:03
✅​ azure linux 283 15 4:25
🔄​ azure windows 2 283 13 4:56
✅​ gcp linux 284 15 5:08
✅​ gcp windows 286 13 4:41
Test Name azure windows
🔄​ TestFsCpDir 🔄​f
🔄​ TestFsCpDir/uc-volumes_to_uc-volumes 🔄​f
Top 3 slowest tests (at least 2 minutes):
duration env testname
4:03 aws windows TestAccept
3:43 gcp windows TestAccept
3:35 azure windows TestAccept

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants