diff --git a/.nextchanges/notable-changes/grants-and-permissions-revoked-on-delete.md b/.nextchanges/notable-changes/grants-and-permissions-revoked-on-delete.md new file mode 100644 index 00000000000..672945b1e92 --- /dev/null +++ b/.nextchanges/notable-changes/grants-and-permissions-revoked-on-delete.md @@ -0,0 +1,12 @@ +Deleting a `grants` or `permissions` block, or emptying it to `[]`, now revokes what it +granted. Both were previously ignored: the grant or permission stayed in place, and so did +anything added outside the bundle. An empty list and a removed block are the same request, +and both are now applied as a delete ([#6474](https://github.com/databricks/cli/pull/6474)). + +`grants: []` revokes everything. `permissions: []` revokes everything but the object owner, +which the API requires on every update. The owner is read from the object rather than assumed +to be whoever deploys, so an object owned by a service principal, or handed over since it was +deployed, keeps the owner it has. + +Nothing changes for a bundle that has always had an empty list, and deleting the resource a +block belongs to is unaffected. diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/databricks.empty.yml.tmpl b/acceptance/bundle/resources/grants/schemas/delete_semantics/databricks.empty.yml.tmpl new file mode 100644 index 00000000000..c111e6825dc --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/databricks.empty.yml.tmpl @@ -0,0 +1,2 @@ +bundle: + name: schema-grants-delete-semantics-$UNIQUE_NAME diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/databricks.yml.tmpl b/acceptance/bundle/resources/grants/schemas/delete_semantics/databricks.yml.tmpl new file mode 100644 index 00000000000..90953232bb3 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/databricks.yml.tmpl @@ -0,0 +1,9 @@ +bundle: + name: schema-grants-delete-semantics-$UNIQUE_NAME + +resources: + schemas: + grants_schema: + name: schema_delete_semantics_$UNIQUE_NAME + catalog_name: main + grants: [{ principal: deco-test-user@databricks.com, privileges: [USE_SCHEMA] }] # GRANTS diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/out.direct.txt b/acceptance/bundle/resources/grants/schemas/delete_semantics/out.direct.txt new file mode 100644 index 00000000000..eb1304606c0 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/out.direct.txt @@ -0,0 +1,50 @@ +=== only the grants node is deleted +delete schemas.grants_schema.grants + +Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.schema_delete_semantics_[UNIQUE_NAME]", + "body": { + "changes": [ + { + "principal": "deco-test-user@databricks.com", + "remove": [ + "ALL_PRIVILEGES" + ] + } + ] + } +} +json = {}; +=== the grants list is empty +delete schemas.grants_schema.grants + +Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.schema_delete_semantics_[UNIQUE_NAME]", + "body": { + "changes": [ + { + "principal": "deco-test-user@databricks.com", + "remove": [ + "ALL_PRIVILEGES" + ] + } + ] + } +} +json = {}; +=== the parent is deleted too +delete schemas.grants_schema +delete schemas.grants_schema.grants + +Plan: 0 to add, 0 to change, 2 to delete, 0 unchanged +{ + "method": "DELETE", + "path": "/api/2.1/unity-catalog/schemas/main.schema_delete_semantics_[UNIQUE_NAME]", + "q": { + "force": "true" + } +} diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/out.terraform.txt b/acceptance/bundle/resources/grants/schemas/delete_semantics/out.terraform.txt new file mode 100644 index 00000000000..e0df82b0cb5 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/out.terraform.txt @@ -0,0 +1,64 @@ +=== only the grants node is deleted +delete schemas.grants_schema.grants + +Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.schema_delete_semantics_[UNIQUE_NAME]", + "body": { + "changes": [ + { + "principal": "deco-test-user@databricks.com", + "remove": [ + "USE_SCHEMA" + ] + } + ] + } +} +json = {}; +=== the grants list is empty +delete schemas.grants_schema.grants + +Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.schema_delete_semantics_[UNIQUE_NAME]", + "body": { + "changes": [ + { + "principal": "deco-test-user@databricks.com", + "remove": [ + "USE_SCHEMA" + ] + } + ] + } +} +json = {}; +=== the parent is deleted too +delete schemas.grants_schema +delete schemas.grants_schema.grants + +Plan: 0 to add, 0 to change, 2 to delete, 0 unchanged +{ + "method": "DELETE", + "path": "/api/2.1/unity-catalog/schemas/main.schema_delete_semantics_[UNIQUE_NAME]", + "q": { + "force": "true" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.schema_delete_semantics_[UNIQUE_NAME]", + "body": { + "changes": [ + { + "principal": "deco-test-user@databricks.com", + "remove": [ + "USE_SCHEMA" + ] + } + ] + } +} diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/out.test.toml b/acceptance/bundle/resources/grants/schemas/delete_semantics/out.test.toml new file mode 100644 index 00000000000..cef45fe553a --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/output.txt b/acceptance/bundle/resources/grants/schemas/delete_semantics/output.txt new file mode 100644 index 00000000000..a20784ffd26 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/output.txt @@ -0,0 +1,27 @@ + +=== Deploy the schema with one grant +>>> [CLI] bundle deploy -qq + +=== Case 1: the grants block is removed, the schema stays +>>> [CLI] bundle deploy -qq + +=== Put the grant back +>>> [CLI] bundle deploy -qq + +=== Case 2: the grants list is set to [] +>>> [CLI] bundle deploy -qq + +=== Put the grant back +>>> [CLI] bundle deploy -qq + +=== Case 3: the schema and its grants are deleted together +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.grants_schema + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.grants_schema + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/schema-grants-delete-semantics-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/script b/acceptance/bundle/resources/grants/schemas/delete_semantics/script new file mode 100644 index 00000000000..4d559a72be9 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/script @@ -0,0 +1,54 @@ +SCHEMA_FULL_NAME=main.schema_delete_semantics_$UNIQUE_NAME +GRANTS_LINE='grants: [{ principal: deco-test-user@databricks.com, privileges: [USE_SCHEMA] }] # GRANTS' + +# The plan, the requests and the leftover grants all go to one per-engine file: the engines +# disagree on how a dropped grants node is classified and on what they send for it. +ENGINE_OUT=out.$DATABRICKS_BUNDLE_ENGINE.txt + +envsubst < databricks.yml.tmpl > databricks.yml +cp databricks.yml databricks.yml.saved + +cleanup() { + errcode $CLI bundle destroy --auto-approve &> LOG.cleanup + rm -f out.requests.txt databricks.yml.saved tmp.yml +} +trap cleanup EXIT + +record() { + echo "=== $1" >> $ENGINE_OUT + $CLI bundle plan >> $ENGINE_OUT + trace $CLI bundle deploy -qq + print_requests.py //unity-catalog --sort >> $ENGINE_OUT + $CLI grants get schema "$SCHEMA_FULL_NAME" | gron.py --noindex | sort_lines.py --repl >> $ENGINE_OUT +} + +restore_grant() { + title "Put the grant back" + cp databricks.yml.saved databricks.yml + trace $CLI bundle deploy -qq + rm -f out.requests.txt +} + +title "Deploy the schema with one grant" +trace $CLI bundle deploy -qq +rm -f out.requests.txt + +title "Case 1: the grants block is removed, the schema stays" +grep -v GRANTS databricks.yml > tmp.yml && mv tmp.yml databricks.yml +record "only the grants node is deleted" + +restore_grant +title "Case 2: the grants list is set to []" +update_file.py databricks.yml "$GRANTS_LINE" 'grants: []' +record "the grants list is empty" + +restore_grant +title "Case 3: the schema and its grants are deleted together" +echo "=== the parent is deleted too" >> $ENGINE_OUT +# Planned against a config with the schema gone, so the plan names both nodes and the +# action each one gets. `bundle destroy` applies the same shape but its banner hides +# child nodes, so it never shows what happens to the grants. +envsubst < databricks.empty.yml.tmpl > databricks.yml +$CLI bundle plan >> $ENGINE_OUT +trace $CLI bundle destroy --auto-approve +print_requests.py //unity-catalog --sort >> $ENGINE_OUT diff --git a/acceptance/bundle/resources/grants/schemas/delete_semantics/test.toml b/acceptance/bundle/resources/grants/schemas/delete_semantics/test.toml new file mode 100644 index 00000000000..6b637eb9111 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/delete_semantics/test.toml @@ -0,0 +1,2 @@ +# Six deploys plus a destroy; extra headroom for heavy parallel runs. +Timeout = '2m' diff --git a/acceptance/bundle/resources/permissions/jobs/revoke_semantics/databricks.yml.tmpl b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/databricks.yml.tmpl new file mode 100644 index 00000000000..7ebacbbca29 --- /dev/null +++ b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/databricks.yml.tmpl @@ -0,0 +1,14 @@ +bundle: + name: job-permissions-revoke-$UNIQUE_NAME + +resources: + jobs: + foo: + name: job_revoke_semantics_$UNIQUE_NAME + tasks: + - task_key: main + notebook_task: + notebook_path: ./notebook.py + permissions: # PERMISSIONS + - level: CAN_VIEW # PERMISSIONS + user_name: deco-test-user@databricks.com # PERMISSIONS diff --git a/acceptance/bundle/resources/permissions/jobs/revoke_semantics/notebook.py b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/notebook.py new file mode 100644 index 00000000000..4914a7436d9 --- /dev/null +++ b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/notebook.py @@ -0,0 +1,2 @@ +# Databricks notebook source +print("hello") diff --git a/acceptance/bundle/resources/permissions/jobs/revoke_semantics/out.test.toml b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/out.test.toml new file mode 100644 index 00000000000..9b8ddfcd2bd --- /dev/null +++ b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/resources/permissions/jobs/revoke_semantics/output.txt b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/output.txt new file mode 100644 index 00000000000..979fe3d941c --- /dev/null +++ b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/output.txt @@ -0,0 +1,57 @@ + +=== Deploy with one grant to a non-owner +>>> [CLI] bundle deploy -qq + +>>> acl +[ + { + "levels": [ + "CAN_VIEW" + ], + "name": "deco-test-user@databricks.com" + }, + { + "levels": [ + "IS_OWNER" + ], + "name": "[USERNAME]" + } +] + +=== Empty the list: everything but the owner is revoked +>>> [CLI] bundle plan +delete jobs.foo.permissions + +Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged + +>>> [CLI] bundle deploy -qq + +>>> acl +[ + { + "levels": [ + "IS_OWNER" + ], + "name": "[USERNAME]" + } +] + +=== Put the grant back, then remove the block entirely: same outcome +>>> [CLI] bundle deploy -qq + +>>> [CLI] bundle plan +delete jobs.foo.permissions + +Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged + +>>> [CLI] bundle deploy -qq + +>>> acl +[ + { + "levels": [ + "IS_OWNER" + ], + "name": "[USERNAME]" + } +] diff --git a/acceptance/bundle/resources/permissions/jobs/revoke_semantics/script b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/script new file mode 100644 index 00000000000..d86406ba556 --- /dev/null +++ b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/script @@ -0,0 +1,39 @@ +envsubst < databricks.yml.tmpl > databricks.yml +cp databricks.yml databricks.yml.saved + +cleanup() { + errcode $CLI bundle destroy --auto-approve &> LOG.cleanup + rm -f out.requests.txt databricks.yml.saved tmp.yml +} +trap cleanup EXIT + +# The API rejects a Set without exactly one IS_OWNER, so an emptied list revokes down to the +# owner rather than to nothing. The testserver does not enforce that, which is why this runs +# on cloud: the read below is the assertion. +acl() { + $CLI api get "/api/2.0/permissions/jobs/$JOB_ID" | + jq -S '[.access_control_list[] + | select(any(.all_permissions[]; .inherited == false)) + | {name: (.user_name // .service_principal_name // .group_name), + levels: [.all_permissions[] | select(.inherited == false) | .permission_level]}]' +} + +title "Deploy with one grant to a non-owner" +trace $CLI bundle deploy -qq +JOB_ID=$(read_id.py foo) +trace acl + +title "Empty the list: everything but the owner is revoked" +update_file.py databricks.yml 'permissions: # PERMISSIONS' 'permissions: []' +sed -i '/# PERMISSIONS/d' databricks.yml +trace $CLI bundle plan +trace $CLI bundle deploy -qq +trace acl + +title "Put the grant back, then remove the block entirely: same outcome" +cp databricks.yml.saved databricks.yml +trace $CLI bundle deploy -qq +sed -i '/# PERMISSIONS/d' databricks.yml +trace $CLI bundle plan +trace $CLI bundle deploy -qq +trace acl diff --git a/acceptance/bundle/resources/permissions/jobs/revoke_semantics/test.toml b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/test.toml new file mode 100644 index 00000000000..2c49e7c7361 --- /dev/null +++ b/acceptance/bundle/resources/permissions/jobs/revoke_semantics/test.toml @@ -0,0 +1,4 @@ +# The API requires exactly one IS_OWNER on every Set, so the revoke floor is owner-only. +# The testserver does not enforce that, so this only means anything against a real workspace. +Cloud = true +EnvMatrix.DMS = [""] diff --git a/bundle/deployplan/action.go b/bundle/deployplan/action.go index e855dbb2197..1ffc6f2797b 100644 --- a/bundle/deployplan/action.go +++ b/bundle/deployplan/action.go @@ -25,6 +25,17 @@ func (a Action) IsChildResource() bool { return len(items) == 4 } +// ParentKey returns the key of the resource a child node hangs off, e.g. +// "resources.schemas.foo" for "resources.schemas.foo.grants". Returns "" if +// resourceKey is not a child node. +func ParentKey(resourceKey string) string { + items := strings.Split(resourceKey, ".") + if len(items) != 4 { + return "" + } + return strings.Join(items[:3], ".") +} + type ActionType string // Actions are ordered in increasing severity. diff --git a/bundle/direct/apply.go b/bundle/direct/apply.go index 6b2fffe9a16..8061cc6da64 100644 --- a/bundle/direct/apply.go +++ b/bundle/direct/apply.go @@ -159,24 +159,9 @@ func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState, return err } - empty, err := d.Adapter.IsEmptyState(newState) + err = d.saveState(db, id, newState, d.DependsOn) if err != nil { - return err - } - - if empty { - // The update emptied the resource out (e.g. all grants revoked). Keeping an entry - // would report the node as tracked-and-unchanged forever, while a fresh deploy of - // the same config plans no node at all; drop it so the two agree. - err = db.DeleteState(d.ResourceKey) - if err != nil { - return fmt.Errorf("deleting state id=%s: %w", id, err) - } - } else { - err = d.saveState(db, id, newState, d.DependsOn) - if err != nil { - return fmt.Errorf("saving state id=%s: %w", id, err) - } + return fmt.Errorf("saving state id=%s: %w", id, err) } waitRemoteState, err := retryOnTransient(ctx, func() (any, error) { diff --git a/bundle/direct/bundle_apply.go b/bundle/direct/bundle_apply.go index 424ae2bdec9..15e3077281a 100644 --- a/bundle/direct/bundle_apply.go +++ b/bundle/direct/bundle_apply.go @@ -43,6 +43,10 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa return } + // Computed up front: the callback below holds a write lock on its own entry, so it + // cannot read another node's entry without risking a lock error. + parentDeleted := willDeleteParent(plan) + g.Run(defaultParallelism, func(resourceKey string, failedDependency *string) bool { entry, err := plan.WriteLockEntry(resourceKey) if err != nil { @@ -100,9 +104,10 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa } if action == deployplan.Delete { - if entry.Gone { - // Planning confirmed the resource is already deleted remotely; only - // remove it from the state, without calling the delete API. + if entry.Gone || parentDeleted[resourceKey] { + // Planning confirmed the resource is already deleted remotely, or the parent + // resource is going away and takes this node with it; only remove it from + // the state, without calling the delete API. err = b.StateDB.DeleteState(resourceKey) } else { err = d.Destroy(ctx, &b.StateDB) @@ -164,6 +169,29 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa }) } +// willDeleteParent returns the child nodes (grants, permissions) whose parent resource the +// same plan deletes: the parent takes them with it, so the child delete is applied as a +// state-only cleanup. +func willDeleteParent(plan *deployplan.Plan) map[string]bool { + result := make(map[string]bool) + + for key, entry := range plan.Plan { + if entry.Action != deployplan.Delete { + continue + } + parent := deployplan.ParentKey(key) + if parent == "" { + continue + } + parentEntry := plan.Plan[parent] + if parentEntry != nil && parentEntry.Action == deployplan.Delete { + result[key] = true + } + } + + return result +} + func (b *DeploymentBundle) LookupReferencePostDeploy(ctx context.Context, path *structpath.PathNode) (any, error) { targetResourceKey, fieldPath := splitResourcePath(path) targetGroup := config.GetResourceTypeFromKey(targetResourceKey) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 453e588187b..92f86f8055f 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -992,16 +992,19 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root return nil, fmt.Errorf("%s: %w", prefix, err) } - // New nodes only: a node with state must stay in the plan, otherwise emptying it plans nothing. - // Apply drops the state entry once the node is empty, so it is skipped from then on. - if _, hasState := db.State[node]; !hasState { - empty, err := adapter.IsEmptyState(newStateConfig) - if err != nil { - return nil, fmt.Errorf("%s: %w", prefix, err) - } - if empty { - continue + // An empty state describes no resource, so the node is not planned as one. A node with + // no state is simply left out: there is nothing to create. A node that has state goes + // back to existingKeys and is planned as a delete, which revokes and stops tracking it - + // emptying the list is the same request as removing the block. + empty, err := adapter.IsEmptyState(newStateConfig) + if err != nil { + return nil, fmt.Errorf("%s: %w", prefix, err) + } + if empty { + if stateEntry, hasState := db.State[node]; hasState { + existingKeys[node] = stateEntry } + continue } // Note, we're extracting references in input config but resolving them in newState.Config which is PrepareState(inputConfig) diff --git a/bundle/direct/dresources/grants.go b/bundle/direct/dresources/grants.go index 5fd10f41046..3bd69befca1 100644 --- a/bundle/direct/dresources/grants.go +++ b/bundle/direct/dresources/grants.go @@ -141,10 +141,38 @@ func (r *ResourceGrants) DoUpdate(ctx context.Context, _ string, state *GrantsSt return nil, err } +// DoDelete revokes every privilege currently assigned on the securable, including any +// granted out of band: dropping the grants node means the empty list is what the bundle +// enforces. Apply only reaches this when the securable itself stays — when the parent is +// deleted too, deleting it takes the grants with it and the node is a state-only cleanup. func (r *ResourceGrants) DoDelete(ctx context.Context, id string, _ *GrantsState) error { - // Similar to permissions, we do nothing there. - // We could delete all grants there, but it would be confusing to explain wrt permissions. - return nil + securableType, fullName, err := parseGrantsID(id) + if err != nil { + return err + } + + assignments, err := r.listGrants(ctx, securableType, fullName) + if err != nil { + return err + } + if len(assignments) == 0 { + return nil + } + + principals := make([]string, 0, len(assignments)) + for _, a := range assignments { + principals = append(principals, a.Principal) + } + slices.Sort(principals) + + _, err = r.client.Grants.Update(ctx, catalog.UpdatePermissions{ + SecurableType: securableType, + FullName: fullName, + Changes: buildGrantChanges(nil, principals), + OmitPermissionsInResponse: false, + ForceSendFields: nil, + }) + return err } func buildGrantChanges(desiredAssignments []catalog.PrivilegeAssignment, removedPrincipals []string) []catalog.PermissionsChange { diff --git a/bundle/direct/dresources/permissions.go b/bundle/direct/dresources/permissions.go index 2ab265d29f1..08def8f4baf 100644 --- a/bundle/direct/dresources/permissions.go +++ b/bundle/direct/dresources/permissions.go @@ -271,7 +271,41 @@ func (r *ResourcePermissions) DoUpdate(ctx context.Context, _ string, newState * // it themselves. Trying to fix permissions back requires // - making assumptions on what it should look like // - storing current user somewhere or storing original permissions somewhere +// IsEmptyState reports an empty permissions list as no resource at all, so emptying it is +// planned as a delete: the same request as removing the block, and the same outcome - every +// permission but the owner revoked. +func (*ResourcePermissions) IsEmptyState(state *PermissionsState) bool { + return len(state.EmbeddedSlice) == 0 +} + +// DoDelete revokes every permission the object carries except its owner, which the API +// requires: a Set without exactly one IS_OWNER is rejected. Apply only reaches this when the +// object itself stays - when the parent is deleted too, deleting it takes the permissions with +// it and the node is a state-only cleanup. +// +// The owner comes from the current permissions rather than the persisted state, which only +// holds what the bundle set and never the owner. Reading it also keeps the owner as it is now, +// which is not always the caller: an object can be owned by a service principal, or have been +// handed over since it was deployed. func (r *ResourcePermissions) DoDelete(ctx context.Context, id string, _ *PermissionsState) error { - // intentional noop - return nil + current, err := r.DoRead(ctx, id) + if err != nil { + return err + } + + ownerOnly := &PermissionsState{ObjectID: id, EmbeddedSlice: nil} + for _, p := range current.EmbeddedSlice { + if p.Level == iam.PermissionLevelIsOwner { + ownerOnly.EmbeddedSlice = append(ownerOnly.EmbeddedSlice, p) + } + } + + // Nothing to revoke down to: an object with no owner takes no Set, and one with only its + // owner is already there. + if len(ownerOnly.EmbeddedSlice) == 0 || len(current.EmbeddedSlice) == len(ownerOnly.EmbeddedSlice) { + return nil + } + + _, err = r.DoUpdate(ctx, id, ownerOnly, nil) + return err }