Skip to content

Fix perpetual permissions drift for a service principal declared under user_name - #6710

Draft
denik wants to merge 4 commits into
mainfrom
denik/analyze-plan
Draft

denik wants to merge 4 commits into
mainfrom
denik/analyze-plan

Conversation

@denik

@denik denik commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Problem

When bundle permissions declare a service principal under user_name (using its application-ID UUID), the Permissions API stores and returns it as service_principal_name. The direct engine's plan then compared desired (user_name) against remote (service_principal_name) and reported a no-op update of the ACL on every run — a false positive that never converged. Reported by a customer after migrating to the direct engine.

Fix

Permission entries are already matched by principal value, so the only difference is the field name. ResourcePermissions.OverrideChangeDesc now treats the same value under user_name vs service_principal_name as the same principal and drops the field-only change; a genuine level change on that principal still reports an update. This is value-based (no UUID sniffing), so GCP name-based service principals are handled too.

OverrideChangeDesc now also receives the desired state (the removal side of the swap needs it); the ~10 sibling implementations gain one unused parameter, and libs/calladapt now accepts nil for interface-kind parameters.

The testserver models the backend's user_name(UUID) → service_principal_name readback, and a new acceptance test asserts the plan converges after deploy and that a real level change is still reported.

This pull request and its description were written by Isaac.

denik and others added 3 commits September 16, 2026 14:48
…r user_name

A service principal listed in bundle permissions under `user_name` (its
application ID) is stored and returned by the Permissions API as
`service_principal_name`, so the direct engine's plan reported a no-op
"update" of the ACL on every run that never converged.

Permission entries are already matched by principal value, so treat the same
value under `user_name` vs `service_principal_name` as one principal and drop
the field-only change; a genuine level change still surfaces. The relaxation
lives in ResourcePermissions.OverrideChangeDesc, which now also receives the
desired state so the removal side of the swap can be recognized.

The testserver models the backend's user_name(UUID) -> service_principal_name
readback, and a new acceptance test asserts the plan converges and that a real
level change is still reported.

Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
… change

Use each resource's concrete state type for the new OverrideChangeDesc
newState parameter instead of `any`. A concrete pointer parameter already
accepts an untyped nil through calladapt's existing pointer path, so the
interface-nil handling added earlier is no longer needed and is reverted.

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

eng-dev-ecosystem-bot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 3cd1c4f

Run: 35103787431

Env 💚​RECOVERED ✅​pass 🙈​skip Time
💚​ aws linux 1 275 16 8:42
💚​ aws windows 1 277 14 6:52
💚​ azure linux 1 274 16 9:25
💚​ azure windows 1 276 14 6:53
💚​ gcp linux 1 275 16 9:57
💚​ gcp windows 1 277 14 8:18
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
💚​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
Top 7 slowest tests (at least 2 minutes):
duration env testname
5:03 azure windows TestAccept
3:44 gcp windows TestAccept
3:15 aws windows TestAccept
2:38 azure windows TestFilerWorkspaceFilesExtensionsReadDir
2:26 azure windows TestFilerRecursiveDelete/workspace_files_extensions
2:16 gcp windows TestFilerWorkspaceNotebook/sqlJupyterNotebook.ipynb
2:01 gcp linux TestFilerWorkspaceFilesExtensionsRead

Co-authored-by: Isaac <no-reply@databricks.com>
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.

2 participants