diff --git a/.nextchanges/bundles/6710.md b/.nextchanges/bundles/6710.md new file mode 100644 index 00000000000..6668d3b0f48 --- /dev/null +++ b/.nextchanges/bundles/6710.md @@ -0,0 +1 @@ +* direct: Fix a spurious `permissions` update reported on every plan and deploy when a service principal is declared under `user_name` and the Permissions API returns it as `service_principal_name`. ([#6710](https://github.com/databricks/cli/pull/6710)) diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/databricks.yml.tmpl b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/databricks.yml.tmpl new file mode 100644 index 00000000000..20915edf952 --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/databricks.yml.tmpl @@ -0,0 +1,18 @@ +bundle: + name: dashboard-sp-perm-$UNIQUE_NAME + +resources: + dashboards: + foo: + display_name: test-dashboard-$UNIQUE_NAME + warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID + serialized_dashboard: '{"pages":[{"name":"page1","displayName":"Page 1"}]}' + permissions: + # A service principal declared under user_name using its application-ID UUID. + # The Permissions API resolves the UUID to a service principal and returns it + # as service_principal_name on GET, so desired (user_name) and remote + # (service_principal_name) name the same principal under different fields. The + # plan matches the entry by value and treats that field difference as no + # change, so the deploy converges instead of reporting a perpetual update. + - level: CAN_MANAGE + user_name: aaaaaaaa-bbbb-4ccc-dddd-eeeeeeeeeeee diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/out.test.toml b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/out.test.toml new file mode 100644 index 00000000000..59b56a2037c --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/out.test.toml @@ -0,0 +1,3 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/output.txt b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/output.txt new file mode 100644 index 00000000000..260fe47741d --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/output.txt @@ -0,0 +1,39 @@ + +=== Deploy the bundle +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dashboard-sp-perm-[UNIQUE_NAME]/default/files... +Created dashboards.foo +Created dashboards.foo.permissions +Files: 4 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== Re-plan after deploy (expected: no changes) +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged + +=== Permissions node of the JSON plan{ + "action": "skip", + "changes": null +} + +=== Lower the permission level: real drift is still detected +>>> [CLI] bundle plan +update dashboards.foo.permissions + +Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged + +=== Permissions node of the JSON plan{ + "action": "update", + "changes": { + "[service_principal_name='[UUID]'].level": { + "action": "update", + "new": "CAN_READ", + "remote": "CAN_MANAGE" + }, + "[user_name='[UUID]'].level": { + "action": "update", + "old": "CAN_MANAGE", + "new": "CAN_READ" + } + } +} diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/script b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/script new file mode 100644 index 00000000000..fec69dab14e --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/script @@ -0,0 +1,23 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +title "Deploy the bundle" +trace $CLI bundle deploy +replace_ids.py + +# The service principal is declared under user_name but the API returns it as +# service_principal_name. The entry is matched by value and the field difference is +# relaxed, so a re-plan reports no changes rather than a perpetual no-op "update". +title "Re-plan after deploy (expected: no changes)" +trace $CLI bundle plan + +title "Permissions node of the JSON plan" +$CLI bundle plan -o json | jq '.plan["resources.dashboards.foo.permissions"] | {action, changes}' + +# A genuine change to the same principal must still be detected: lowering the level +# leaves the principal-field difference relaxed but reports the level update. +title "Lower the permission level: real drift is still detected" +update_file.py databricks.yml "CAN_MANAGE" "CAN_READ" +trace $CLI bundle plan + +title "Permissions node of the JSON plan" +$CLI bundle plan -o json | jq '.plan["resources.dashboards.foo.permissions"] | {action, changes}' diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/test.toml b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/test.toml new file mode 100644 index 00000000000..7c25f8ee2f3 --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/test.toml @@ -0,0 +1,6 @@ +# Reproduction of a false-positive permissions "update" after migrating to the +# direct engine. Direct-only: the customer hit this on the direct engine and the +# JSON plan format is direct-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [".databricks", "databricks.yml"] diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index baa5aa8d1eb..a4e0ceb6408 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -514,7 +514,7 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change savedAction := ch.Action savedReason := ch.Reason - err = adapter.OverrideChangeDesc(ctx, path, ch, remoteState) + err = adapter.OverrideChangeDesc(ctx, path, ch, newState, remoteState) if err != nil { return fmt.Errorf("internal error: failed to classify change: %w", err) } diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index 6210a952112..9f887c66cf7 100644 --- a/bundle/direct/dresources/adapter.go +++ b/bundle/direct/dresources/adapter.go @@ -70,7 +70,8 @@ type IResource interface { DoDelete(ctx context.Context, id string, state any) error // [Optional] OverrideChangeDesc can implement custom logic to update a given ChangeDesc; it is run last after built-in classifiers and field triggers. - OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, changedesc *ChangeDesc, remoteState any) error + // newState is the desired (config) state and remoteState is the value read from the backend. + OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, changedesc *ChangeDesc, newState, remoteState any) error // DoCreate creates a new resource from the newState. Returns id of the resource and optionally remote state. // If remote state is available as part of the operation, return it; otherwise return nil. @@ -689,8 +690,8 @@ func (a *Adapter) HasOverrideChangeDesc() bool { } // OverrideChangeDesc allows custom logic to override change classification. -func (a *Adapter) OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, change *ChangeDesc, remoteState any) error { - _, err := a.overrideChangeDesc.Call(ctx, path, change, remoteState) +func (a *Adapter) OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, change *ChangeDesc, newState, remoteState any) error { + _, err := a.overrideChangeDesc.Call(ctx, path, change, newState, remoteState) return err } diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 28170a4c02d..a757b38eff8 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1226,7 +1226,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W require.NoError(t, err) if adapter.HasOverrideChangeDesc() { - err = adapter.OverrideChangeDesc(ctx, p, &deployplan.ChangeDesc{}, nil) + err = adapter.OverrideChangeDesc(ctx, p, &deployplan.ChangeDesc{}, nil, nil) require.NoError(t, err) } diff --git a/bundle/direct/dresources/app.go b/bundle/direct/dresources/app.go index 066669f8b87..1904c772e97 100644 --- a/bundle/direct/dresources/app.go +++ b/bundle/direct/dresources/app.go @@ -258,7 +258,7 @@ func hasAppChanges(entry *PlanEntry) bool { // git_source) while the app has no active deployment. DoRead reads them only from the // active deployment, so before the first deploy (or once a stop clears it) the remote // side is empty and the diff is spurious; it applies on the next start (manageLifecycle). -func (*ResourceApp) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, remote *AppRemote) error { +func (*ResourceApp) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *AppState, remote *AppRemote) error { // Prefix(1) so a nested diff (e.g. config.command) matches its top-level field. switch path.Prefix(1).String() { case "source_code_path", "config", "git_source": diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index 0605d50b86c..13530810ed2 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -285,7 +285,7 @@ func (r *ResourceCluster) DoDelete(ctx context.Context, id string, _ *ClusterSta return r.client.Clusters.PermanentDeleteByClusterId(ctx, id) } -func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath.PathNode, change *ChangeDesc, remoteState *ClusterRemote) error { +func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath.PathNode, change *ChangeDesc, _ *ClusterState, remoteState *ClusterRemote) error { path := p.Prefix(1).String() // Remaining overrides only apply to Update actions. diff --git a/bundle/direct/dresources/dashboard.go b/bundle/direct/dresources/dashboard.go index cdfd57c5623..1edc4fe5bfb 100644 --- a/bundle/direct/dresources/dashboard.go +++ b/bundle/direct/dresources/dashboard.go @@ -398,7 +398,7 @@ func (r *ResourceDashboard) DoDelete(ctx context.Context, id string, _ *Dashboar }) } -func (r *ResourceDashboard) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *DashboardState) error { +func (r *ResourceDashboard) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _, _ *DashboardState) error { switch path.String() { case "etag": // change.New is always nil for etag because it's not present in the config diff --git a/bundle/direct/dresources/experiment.go b/bundle/direct/dresources/experiment.go index e4f2e8ebbd7..ef46ce3d8da 100644 --- a/bundle/direct/dresources/experiment.go +++ b/bundle/direct/dresources/experiment.go @@ -83,7 +83,7 @@ func (r *ResourceExperiment) DoDelete(ctx context.Context, id string, _ *ml.Crea // // This matches the Terraform provider's experimentNameSuppressDiff behavior. // https://github.com/databricks/terraform-provider-databricks/blob/8945a7b2328659b1fc976d04e32457305860131f/mlflow/resource_mlflow_experiment.go#L13 -func (*ResourceExperiment) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *ml.Experiment) error { +func (*ResourceExperiment) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *ml.CreateExperiment, _ *ml.Experiment) error { if change.Action == deployplan.Skip { return nil } diff --git a/bundle/direct/dresources/genie_space.go b/bundle/direct/dresources/genie_space.go index 21d2c632409..cd139a748e0 100644 --- a/bundle/direct/dresources/genie_space.go +++ b/bundle/direct/dresources/genie_space.go @@ -214,7 +214,7 @@ func (r *ResourceGenieSpace) DoUpdate(ctx context.Context, id string, config *re // OverrideChangeDesc handles the etag field. The user never sets it directly; // we compare the stored etag against the remote one and Skip if they match. // This mirrors ResourceDashboard.OverrideChangeDesc. -func (r *ResourceGenieSpace) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *resources.GenieSpaceConfig) error { +func (r *ResourceGenieSpace) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _, _ *resources.GenieSpaceConfig) error { switch path.String() { case "etag": // change.New is always nil for etag because it's not present in the diff --git a/bundle/direct/dresources/genie_space_test.go b/bundle/direct/dresources/genie_space_test.go index f77289083d3..23bedd98256 100644 --- a/bundle/direct/dresources/genie_space_test.go +++ b/bundle/direct/dresources/genie_space_test.go @@ -182,20 +182,20 @@ func TestGenieSpaceOverrideChangeDescEtag(t *testing.T) { t.Run("Skip when stored matches remote", func(t *testing.T) { change := &ChangeDesc{Old: "etag-7", Remote: "etag-7"} - require.NoError(t, r.OverrideChangeDesc(t.Context(), etagPath, change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), etagPath, change, nil, nil)) assert.Equal(t, deployplan.Skip, change.Action) }) t.Run("Update when stored differs from remote", func(t *testing.T) { change := &ChangeDesc{Old: "etag-7", Remote: "etag-8"} - require.NoError(t, r.OverrideChangeDesc(t.Context(), etagPath, change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), etagPath, change, nil, nil)) assert.Equal(t, deployplan.Update, change.Action) }) t.Run("Other paths are untouched", func(t *testing.T) { titlePath := structpath.MustParsePath("title") change := &ChangeDesc{Action: deployplan.Update, Old: "a", Remote: "b"} - require.NoError(t, r.OverrideChangeDesc(t.Context(), titlePath, change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), titlePath, change, nil, nil)) assert.Equal(t, deployplan.Update, change.Action) }) } diff --git a/bundle/direct/dresources/job_run.go b/bundle/direct/dresources/job_run.go index 36031ec612e..86eec3fad6a 100644 --- a/bundle/direct/dresources/job_run.go +++ b/bundle/direct/dresources/job_run.go @@ -370,7 +370,7 @@ func reportRunLine(ctx context.Context, runID int64, msg string) { // Removing a trigger drops the change and leaves the last fingerprint in state, // so re-adding the trigger only re-fires when the watched files changed // meanwhile. All other trigger changes recreate the run. -func (*ResourceJobRun) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, remote *JobRunRemote) error { +func (*ResourceJobRun) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *JobRunState, remote *JobRunRemote) error { switch path.String() { case "lifecycle": // Dropped when the trigger is removed (New nil) and when both sides are diff --git a/bundle/direct/dresources/job_run_test.go b/bundle/direct/dresources/job_run_test.go index 5da8feb39e0..8799e1cbaf2 100644 --- a/bundle/direct/dresources/job_run_test.go +++ b/bundle/direct/dresources/job_run_test.go @@ -500,7 +500,7 @@ func TestJobRunOverrideChangeDescTriggerRemoved(t *testing.T) { } { t.Run(tt.name, func(t *testing.T) { change := &ChangeDesc{Action: deployplan.Recreate, Old: tt.old, New: tt.new} - require.NoError(t, r.OverrideChangeDesc(t.Context(), structpath.MustParsePath(tt.path), change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), structpath.MustParsePath(tt.path), change, nil, nil)) assert.Equal(t, tt.action, change.Action) assert.Equal(t, tt.reason, change.Reason) }) diff --git a/bundle/direct/dresources/permissions.go b/bundle/direct/dresources/permissions.go index c2aebb4d0ac..127c7d49d36 100644 --- a/bundle/direct/dresources/permissions.go +++ b/bundle/direct/dresources/permissions.go @@ -6,6 +6,8 @@ import ( "reflect" "strings" + "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/structs/structvar" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/iam" @@ -167,6 +169,77 @@ func (*ResourcePermissions) KeyedSlices() map[string]any { } } +// hasPrincipal reports whether any entry names the given principal value under the +// given field ("user_name" or "service_principal_name"). Nil-safe: missing state +// (e.g. a resource not yet created) holds no principals. +func (s *PermissionsState) hasPrincipal(field, value string) bool { + if s == nil { + return false + } + for _, p := range s.EmbeddedSlice { + switch field { + case "user_name": + if p.UserName == value { + return true + } + case "service_principal_name": + if p.ServicePrincipalName == value { + return true + } + } + } + return false +} + +// OverrideChangeDesc suppresses false-positive drift when a principal is declared +// under one field (e.g. user_name) but the Permissions API stores and returns it +// under the other (service_principal_name) — which the backend does when a user_name +// holds a service principal's application ID. The entry is matched by value (see +// permissionKey), so the only diff is the field name; we drop the user_name / +// service_principal_name leaf change whenever the same value is present under the +// counterpart field on the other side. A level change on the same principal surfaces +// via its own ".level" leaf and is left untouched. +// +// Matching purely on value means a user and a service principal sharing the exact +// same name string would be treated as one principal; that collision does not occur +// in practice (user names are emails, service principal names are application IDs). +func (*ResourcePermissions) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, ch *ChangeDesc, newState, remoteState *PermissionsState) error { + field, ok := path.StringKey() + if !ok { + return nil + } + + var counterpart string + switch field { + case "user_name": + counterpart = "service_principal_name" + case "service_principal_name": + counterpart = "user_name" + default: + return nil + } + + // Match on value only, not level: level is a separate leaf that surfaces on its + // own. Gating these drops on a matching level would resurrect the field-swap diff + // whenever the level also changed. + + // Config adds this field while remote holds the same value under the counterpart. + if newStr, ok := ch.New.(string); ok && newStr != "" && remoteState.hasPrincipal(counterpart, newStr) { + ch.Reason = deployplan.ReasonDrop + return nil + } + + // Remote has this field (config does not) while config declares the same value + // under the counterpart field. + if ch.New == nil { + if remoteStr, ok := ch.Remote.(string); ok && remoteStr != "" && newState.hasPrincipal(counterpart, remoteStr) { + ch.Reason = deployplan.ReasonDrop + } + } + + return nil +} + // parsePermissionsID extracts the object type and ID from a permissions ID string. // Handles both 3-part IDs ("/jobs/123") and 4-part IDs ("/sql/warehouses/uuid"). func parsePermissionsID(id string) (extractedType, extractedID string, err error) { diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index aa9aae7ee33..7d3c8ae4eb6 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -233,7 +233,7 @@ func (r *ResourcePipeline) DoDelete(ctx context.Context, id string, state *Pipel // old=false, new=nil, remote=nil. // The problem is that classifier skips the change if remote and new are "empty". There is a bug // where "false" value for a boolean field is treated as empty. Hence, we set this override. -func (*ResourcePipeline) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *PipelineRemote) error { +func (*ResourcePipeline) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *PipelineState, _ *PipelineRemote) error { if path.String() == "cascade_on_destroy" && !structdiff.IsEqual(change.Old, change.New) { change.Action = deployplan.Update } diff --git a/bundle/direct/dresources/pipeline_test.go b/bundle/direct/dresources/pipeline_test.go index 3629255a600..c768c17493a 100644 --- a/bundle/direct/dresources/pipeline_test.go +++ b/bundle/direct/dresources/pipeline_test.go @@ -30,26 +30,26 @@ func TestPipelineOverrideChangeDescCascadeOnDestroy(t *testing.T) { t.Run("unset forces state-only update", func(t *testing.T) { change := &ChangeDesc{Action: deployplan.Skip, Old: ptr(false), New: nil} - require.NoError(t, r.OverrideChangeDesc(t.Context(), cascadePath, change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), cascadePath, change, nil, nil)) assert.Equal(t, deployplan.Update, change.Action) }) t.Run("changed value forces state-only update", func(t *testing.T) { change := &ChangeDesc{Action: deployplan.Skip, Old: ptr(true), New: ptr(false)} - require.NoError(t, r.OverrideChangeDesc(t.Context(), cascadePath, change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), cascadePath, change, nil, nil)) assert.Equal(t, deployplan.Update, change.Action) }) t.Run("unchanged value is left as skip", func(t *testing.T) { change := &ChangeDesc{Action: deployplan.Skip, Old: ptr(false), New: ptr(false)} - require.NoError(t, r.OverrideChangeDesc(t.Context(), cascadePath, change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), cascadePath, change, nil, nil)) assert.Equal(t, deployplan.Skip, change.Action) }) t.Run("other paths are untouched", func(t *testing.T) { namePath := structpath.MustParsePath("name") change := &ChangeDesc{Action: deployplan.Skip, Old: "a", New: nil} - require.NoError(t, r.OverrideChangeDesc(t.Context(), namePath, change, nil)) + require.NoError(t, r.OverrideChangeDesc(t.Context(), namePath, change, nil, nil)) assert.Equal(t, deployplan.Skip, change.Action) }) } diff --git a/bundle/direct/dresources/secret.go b/bundle/direct/dresources/secret.go index 122ea0440e9..10677207d6f 100644 --- a/bundle/direct/dresources/secret.go +++ b/bundle/direct/dresources/secret.go @@ -125,7 +125,7 @@ func (r *ResourceSecret) DoDelete(ctx context.Context, id string, _ *catalog.Sec // the actual stored value. We compare new vs remote (via effective_value from DoRead) // to decide whether the secret actually changed: if they are equal, the user's config // already matches what is stored remotely and no update is needed. -func (*ResourceSecret) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, ch *ChangeDesc, _ *catalog.Secret) error { +func (*ResourceSecret) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, ch *ChangeDesc, _, _ *catalog.Secret) error { if path.String() != "value" { return nil } diff --git a/bundle/direct/dresources/vector_search_index.go b/bundle/direct/dresources/vector_search_index.go index 39e29cb7b8e..54eb65a1b5f 100644 --- a/bundle/direct/dresources/vector_search_index.go +++ b/bundle/direct/dresources/vector_search_index.go @@ -249,7 +249,7 @@ func (r *ResourceVectorSearchIndex) WaitAfterDelete(ctx context.Context, id stri // lookupEndpointUuid distinguishes this (404 -> "") from transient errors // (propagated through DoRead/DoCreate), so reaching this branch with empty // remoteUuid unambiguously means the endpoint is gone. -func (*ResourceVectorSearchIndex) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, remote *VectorSearchIndexRemote) error { +func (*ResourceVectorSearchIndex) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *VectorSearchIndexState, remote *VectorSearchIndexRemote) error { if path.String() != "endpoint_uuid" { return nil } diff --git a/libs/testserver/permissions.go b/libs/testserver/permissions.go index a3584c1d49a..ab13d167721 100644 --- a/libs/testserver/permissions.go +++ b/libs/testserver/permissions.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/databricks/databricks-sdk-go/service/iam" + "github.com/google/uuid" ) // source: https://github.com/databricks/terraform-provider-databricks/blob/main/permissions/permission_definitions.go @@ -39,6 +40,13 @@ var requestObjectTypeToObjectType = map[string]string{ "alertsv2": "alertv2", } +// isServicePrincipalID reports whether name is a service principal application ID +// (a UUID). The Permissions API treats a UUID-valued user_name as a service principal. +func isServicePrincipalID(name string) bool { + _, err := uuid.Parse(name) + return err == nil +} + // aclPrincipalKey returns a unique key identifying the principal in an ACL entry. func aclPrincipalKey(acl iam.AccessControlResponse) string { switch { @@ -269,15 +277,27 @@ func (s *FakeWorkspace) SetPermissions(req Request) any { // Convert AccessControlRequest to AccessControlResponse and replace the ACL. existingPermissions.AccessControlList = nil for _, acl := range updateRequest.AccessControlList { - display := acl.UserName + userName := acl.UserName + servicePrincipalName := acl.ServicePrincipalName + + // The real Permissions API resolves a user_name that is actually a service + // principal's application ID (a UUID) to a service principal, and returns it + // as service_principal_name on GET. Model that here so a bundle that declares + // a service principal under user_name converges the same way it does on cloud. + if userName != "" && isServicePrincipalID(userName) { + servicePrincipalName = userName + userName = "" + } + + display := userName if display == "" { - display = acl.ServicePrincipalName + display = servicePrincipalName } response := iam.AccessControlResponse{ - UserName: acl.UserName, + UserName: userName, GroupName: acl.GroupName, - ServicePrincipalName: acl.ServicePrincipalName, + ServicePrincipalName: servicePrincipalName, DisplayName: display, }