Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .nextchanges/bundles/grants-all-privileges.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
Fixes [#6030](https://github.com/databricks/cli/issues/6030): spurious `update` on catalog/schema/volume grants (direct engine); a principal granted `ALL_PRIVILEGES` no longer drifts when the backend also reports the concrete privileges it implies ([#6064](https://github.com/databricks/cli/pull/6064)).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
bundle:
name: schema-grants-all-privileges-coexist-$UNIQUE_NAME

resources:
schemas:
grants_schema:
name: schema_all_priv_coexist_$UNIQUE_NAME
catalog_name: main
grants:
# Config declares only ALL_PRIVILEGES for the principal. Reproduces
# issue #6030: the backend also reports a concrete privilege for this
# principal (added out of band below / materialized for owners), and
# buildGrantChanges skips the ALL_PRIVILEGES removal, so the concrete
# privilege is never reconciled and the plan reports perpetual update.
- principal: $CURRENT_USER_NAME
privileges:
- ALL_PRIVILEGES
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@

>>> [CLI] bundle plan
Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@

>>> [CLI] bundle plan
update schemas.grants_schema.grants

Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/schema-grants-all-privileges-coexist-[UNIQUE_NAME]/default/files...
Deploying resources...
Updating deployment state...
Deployment complete!

>>> [CLI] grants update schema main.schema_all_priv_coexist_[UNIQUE_NAME] --json @update.resolved.json

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/schema-grants-all-privileges-coexist-[UNIQUE_NAME]/default/files...
Deploying resources...
Updating deployment state...
Deployment complete!

>>> [CLI] bundle plan
Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged

>>> errcode [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-all-privileges-coexist-[UNIQUE_NAME]/default

Deleting files...
Destroy complete!
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
SCHEMA_FULL_NAME=main.schema_all_priv_coexist_$UNIQUE_NAME

envsubst < databricks.yml.tmpl > databricks.yml

cleanup() {
trace errcode $CLI bundle destroy --auto-approve
rm -f out.requests.txt
}
trap cleanup EXIT

trace $CLI bundle deploy

# Out of band, grant a concrete privilege in addition to the ALL_PRIVILEGES that
# the config declares for the same principal. This reproduces issue #6030: the
# backend then reports both ALL_PRIVILEGES and the concrete privilege.
envsubst < update.json > update.resolved.json
trace $CLI grants update schema "$SCHEMA_FULL_NAME" --json @update.resolved.json > /dev/null

# The first plan differs between engines: the direct engine treats ALL_PRIVILEGES
# as implying every concrete privilege, so it reports no drift (issue #6030),
# while terraform revokes the extra privilege to match the config exactly.
trace $CLI bundle plan > out.plan.$DATABRICKS_BUNDLE_ENGINE.txt 2>&1

# After deploying, both engines converge: the follow-up plan reports no changes.
trace $CLI bundle deploy
trace $CLI bundle plan
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
Ignore = ['update.resolved.json']
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
{
"changes": [
{
"principal": "$CURRENT_USER_NAME",
"add": ["USE_SCHEMA"]
}
]
}
26 changes: 22 additions & 4 deletions bundle/direct/dresources/grants.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,10 +48,9 @@ func PrepareGrantsInputConfig(inputConfig any, node string) (*structvar.StructVa
return nil, fmt.Errorf("expected *[]catalog.PrivilegeAssignment, got %T", inputConfig)
}

// Backend sorts privileges, so we sort here as well.
for i := range *grantsPtr {
slices.Sort((*grantsPtr)[i].Privileges)
}
// Normalize the same way as DoRead (sort, collapse ALL_PRIVILEGES) so the
// config and the value read back compare equal.
normalizeAssignments(*grantsPtr)

return &structvar.StructVar{
Value: &GrantsState{
Expand DownExpand Up@@ -222,9 +221,28 @@ func (r *ResourceGrants) listGrants(ctx context.Context, securableType, fullName
}
pageToken = resp.NextPageToken
}
// Normalize the same way as the config side (sort, collapse ALL_PRIVILEGES)
// so the two compare equal and we don't report false drift.
normalizeAssignments(assignments)
return assignments, nil
}

// normalizeAssignments sorts each assignment's privileges (the backend sorts
// them, so we match that) and collapses a principal holding ALL_PRIVILEGES down
// to just ALL_PRIVILEGES. The collapse is applied to both the config and read
// sides, so config granting only ALL_PRIVILEGES matches a backend that reports
// ALL_PRIVILEGES plus the concrete privileges it implies, instead of reporting a
// perpetual update.
func normalizeAssignments(assignments []catalog.PrivilegeAssignment) {
for i := range assignments {
if slices.Contains(assignments[i].Privileges, catalog.PrivilegeAllPrivileges) {
assignments[i].Privileges = []catalog.Privilege{catalog.PrivilegeAllPrivileges}
continue
}
slices.Sort(assignments[i].Privileges)
}
}

func extractGrantResourceType(node string) (string, error) {
rest, ok := strings.CutPrefix(node, "resources.")
if !ok {
Expand Down
38 changes: 38 additions & 0 deletions bundle/direct/dresources/grants_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,3 +74,41 @@ func TestBuildGrantChanges(t *testing.T) {
})
}
}

func TestNormalizeAssignments(t *testing.T) {
tests := []struct {
name string
input []catalog.PrivilegeAssignment
expected []catalog.PrivilegeAssignment
}{
{
name: "sorts privileges",
input: []catalog.PrivilegeAssignment{
{Principal: "alice", Privileges: []catalog.Privilege{catalog.PrivilegeUseSchema, catalog.PrivilegeApplyTag}},
},
expected: []catalog.PrivilegeAssignment{
{Principal: "alice", Privileges: []catalog.Privilege{catalog.PrivilegeApplyTag, catalog.PrivilegeUseSchema}},
},
},
{
// Regression test for #6030: ALL_PRIVILEGES implies every concrete
// privilege, so a principal holding it collapses to just
// ALL_PRIVILEGES. Applied to both config and remote, this stops the
// backend's extra concrete privileges from showing as drift.
name: "collapses ALL_PRIVILEGES with concrete privileges",
input: []catalog.PrivilegeAssignment{
{Principal: "alice", Privileges: []catalog.Privilege{catalog.PrivilegeUseCatalog, catalog.PrivilegeAllPrivileges, catalog.PrivilegeCreateSchema}},
},
expected: []catalog.PrivilegeAssignment{
{Principal: "alice", Privileges: []catalog.Privilege{catalog.PrivilegeAllPrivileges}},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
normalizeAssignments(tt.input)
assert.Equal(t, tt.expected, tt.input)
})
}
}
Loading