direct: revoke grants and permissions when the block is deleted or emptied - #6474
Draft
shreyas-goenka wants to merge 7 commits into
Draft
direct: revoke grants and permissions when the block is deleted or emptied#6474shreyas-goenka wants to merge 7 commits into
shreyas-goenka wants to merge 7 commits into
Conversation
…stays DoDelete for grants was a deliberate no-op, so removing a grants block left every grant in place. It now revokes what is actually assigned, including out-of-band grants, because dropping the node means the empty list is what the bundle enforces. The no-op existed to keep `bundle destroy` working, so apply now distinguishes the two triggers: when the parent resource is deleted in the same plan it takes the child with it, and the child delete is applied as a state-only cleanup instead. Co-authored-by: Isaac <no-reply@databricks.com>
Replaces the unit tests with an acceptance test that records the plan, the requests each deploy sends and the grants left on the schema for all three cases: the grants block removed, the list set to [], and the schema deleted along with its grants. The goldens are per-engine because the engines still disagree on the second case (direct plans an update, terraform a delete) and on the third, where terraform sends a redundant revoke for a securable it is about to drop. Also shortens childDeletesWithDeletedParent to willDeleteParent. Co-authored-by: Isaac <no-reply@databricks.com>
exhaustruct wants every field of catalog.UpdatePermissions listed, matching DoUpdate. The test wrote 16 golden files, one per case per engine. All three cases now append to a single per-engine file, so the whole comparison reads top to bottom in one place and the directory holds two goldens instead of sixteen. Co-authored-by: Isaac <no-reply@databricks.com>
`bundle destroy` hides child nodes from its banner, so the case where the parent goes away never showed what the grants node was planned as. Planning against a config with the schema removed names both nodes and their action, which is the same shape destroy applies: both are a delete, and only the parent's reaches the API on direct. Co-authored-by: Isaac <no-reply@databricks.com>
shreyas-goenka
marked this pull request as ready for review
September 2, 2026 02:24
Contributor
Approval status: pending
|
shreyas-goenka
requested review from
denik and
pietern
and removed request for
denik
September 2, 2026 11:14
shreyas-goenka
marked this pull request as draft
September 3, 2026 12:43
An empty list and a removed block are the same request, so they now get the same action. Emptying a node planned an update that revoked and then dropped the state entry, so the next plan saw no state and skipped the node - the list stopped being enforced after the first deploy, and out-of-band grants survived. It also left apply reporting a resource that no longer had state. The node goes back to existingKeys instead, which plans a delete: DoDelete revokes, and the entry goes away because the node really is gone. Terraform already classified it this way, so the engines now agree. Co-authored-by: Isaac <no-reply@databricks.com>
Emptying a permissions list, or removing the block, was silently ignored: DoDelete did nothing, so a permission the bundle had granted stayed granted, and anything added out of band stayed too. Both now revoke, which is what the config asks for. The owner survives because the API insists on it: a Set without exactly one IS_OWNER is rejected, so owner-only is the floor rather than an empty list. It is read from the current permissions instead of assumed to be the caller - the persisted state never holds the owner, and an object can be owned by a service principal or have been handed over since it was deployed. IsEmptyState makes an emptied list take the same path as a removed block, matching grants. Nothing changes for a first deploy that starts out empty: that node has no state and is still left unplanned. The parent-deleted case needs nothing new - the plan already applies a child delete as a state-only cleanup when the parent goes away in the same run. Co-authored-by: Isaac <no-reply@databricks.com>
It changes what a deploy does to grants and permissions that are already there, including ones the bundle did not create, so it belongs above the per-area sections rather than in two separate entries under Bundles. Co-authored-by: Isaac <no-reply@databricks.com>
shreyas-goenka
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deleting a
grantsorpermissionsblock, or emptying it to[], now revokes what it granted. Both were silently ignored before, so a grant or permission stayed in place — including anything added outside the bundle.An empty list and a removed block are the same request, so both are applied as a delete.
grants: []revokes everything;permissions: []leaves the owner, which the API requires on every update, read from the object rather than assumed to be whoever deploys.Unaffected: a bundle that has always had an empty list, and deleting the resource a block belongs to.
Verified against a real workspace: grants suite 38/38, plus new cloud tests for both resources.
This pull request and its description were written by Isaac.