direct: Fix permanent drift on permissions after out-of-band parent recreate

NEXT_CHANGELOG.md

@@ -12,6 +12,7 @@
 * direct: Fix resolving a resource reference that is used more than once within the same field ([#5558](https://github.com/databricks/cli/pull/5558)).
 * Bundle variable references now accept Unicode letters in path segments (e.g. `${var.变量}`). ([#5532](https://github.com/databricks/cli/pull/5532))
 * Ignore remote changes for vector search direct_access_index_spec.schema_json to prevent drift when the backend normalizes the schema ([#5481](https://github.com/databricks/cli/pull/5481)).
+* direct: Fix permanent drift on `permissions` when the parent resource is deleted and recreated out-of-band with the same name ([#5587](https://github.com/databricks/cli/pull/5587)).
 
 ### Dependency updates
 

acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/databricks.yml.tmpl

@@ -0,0 +1,13 @@
+bundle:
+  name: drift-mse-recreated-same-name-$UNIQUE_NAME
+
+sync:
+  paths: []
+
+resources:
+  model_serving_endpoints:
+    my_endpoint:
+      name: mse-endpoint-$UNIQUE_NAME
+      permissions:
+        - level: CAN_VIEW
+          user_name: deco-test-user@databricks.com

acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/output.txt

@@ -0,0 +1,61 @@
+
+=== Initial deployment
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/drift-mse-recreated-same-name-[UNIQUE_NAME]/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> [CLI] serving-endpoints get mse-endpoint-[UNIQUE_NAME]
+{
+  "name": "mse-endpoint-[UNIQUE_NAME]",
+  "creator": "[USERNAME]"
+}
+
+=== Delete and recreate remotely with the same name
+>>> [CLI] serving-endpoints delete mse-endpoint-[UNIQUE_NAME]
+
+>>> [CLI] serving-endpoints create mse-endpoint-[UNIQUE_NAME] --no-wait
+{
+  "name": "mse-endpoint-[UNIQUE_NAME]",
+  "creator": "[USERNAME]"
+}
+
+>>> [CLI] serving-endpoints get mse-endpoint-[UNIQUE_NAME]
+{
+  "name": "mse-endpoint-[UNIQUE_NAME]",
+  "creator": "[USERNAME]"
+}
+Original endpoint id: [ORIGINAL_ENDPOINT_ID]
+Remote recreated endpoint id: [REMOTE_RECREATED_ENDPOINT_ID]
+
+=== Plan after out-of-band recreate
+>>> [CLI] bundle plan
+update model_serving_endpoints.my_endpoint.permissions
+
+Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/drift-mse-recreated-same-name-[UNIQUE_NAME]/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> [CLI] serving-endpoints get mse-endpoint-[UNIQUE_NAME]
+{
+  "name": "mse-endpoint-[UNIQUE_NAME]",
+  "creator": "[USERNAME]"
+}
+
+=== Verify no permanent drift after deploy
+>>> [CLI] bundle plan
+Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged
+
+>>> [CLI] bundle destroy --auto-approve
+The following resources will be deleted:
+  delete resources.model_serving_endpoints.my_endpoint
+
+All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/drift-mse-recreated-same-name-[UNIQUE_NAME]/default
+
+Deleting files...
+Destroy complete!

acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/script

@@ -0,0 +1,41 @@
+envsubst < databricks.yml.tmpl > databricks.yml
+
+cleanup() {
+    trace $CLI bundle destroy --auto-approve
+    rm -f out.requests.txt
+}
+trap cleanup EXIT
+
+endpoint_name="mse-endpoint-${UNIQUE_NAME}"
+
+title "Initial deployment"
+trace $CLI bundle deploy
+
+original_endpoint_id=$($CLI serving-endpoints get "${endpoint_name}" | jq -r '.id')
+add_repl.py "$original_endpoint_id" "ORIGINAL_ENDPOINT_ID"
+trace $CLI serving-endpoints get "${endpoint_name}" | jq '{name, creator}'
+
+title "Delete and recreate remotely with the same name"
+trace $CLI serving-endpoints delete "${endpoint_name}"
+trace $CLI serving-endpoints create "${endpoint_name}" --no-wait | jq '{name, creator}'
+
+remote_recreated_endpoint_id=$($CLI serving-endpoints get "${endpoint_name}" | jq -r '.id')
+add_repl.py "$remote_recreated_endpoint_id" "REMOTE_RECREATED_ENDPOINT_ID"
+trace $CLI serving-endpoints get "${endpoint_name}" | jq '{name, creator}'
+
+printf "Original endpoint id: %s\n" "$original_endpoint_id"
+printf "Remote recreated endpoint id: %s\n" "$remote_recreated_endpoint_id"
+
+if [ "$original_endpoint_id" = "$remote_recreated_endpoint_id" ]; then
+    echo "Expected remote recreation to assign a different endpoint id" >&2
+    exit 1
+fi
+
+title "Plan after out-of-band recreate"
+trace $CLI bundle plan
+
+trace $CLI bundle deploy
+trace $CLI serving-endpoints get "${endpoint_name}" | jq '{name, creator}'
+
+title "Verify no permanent drift after deploy"
+trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete"

acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/test.toml

@@ -0,0 +1,8 @@
+Badness = "After deleting and recreating a model serving endpoint remotely with the same name but a different endpoint_id, bundle plan shows an update on permissions (instead of create as in V2) because the V1 permissions API does not delete ACLs immediately when the parent is gone. UpdateWithID persists the new object_id in deployment state so subsequent plans do not show permanent drift."
+
+Local = true
+Cloud = true
+RequiresUnityCatalog = true
+RecordRequests = false
+
+EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]

acceptance/bundle/resources/vector_search_endpoints/drift/recreated_same_name/output.txt

@@ -34,9 +34,9 @@ Remote recreated endpoint UUID: [REMOTE_RECREATED_ENDPOINT_UUID]
 
 === Plan after out-of-band recreate
 >>> [CLI] bundle plan
-create vector_search_endpoints.my_endpoint.permissions
+update vector_search_endpoints.my_endpoint.permissions
 
-Plan: 1 to add, 0 to change, 0 to delete, 1 unchanged
+Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged
 
 >>> [CLI] bundle deploy
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/drift-vs-endpoint-recreated-same-name-[UNIQUE_NAME]/default/files...

acceptance/bundle/state/permission_level_migration/out.plan.direct.json

@@ -29,7 +29,7 @@
           "label": "${resources.jobs.my_job.id}"
         }
       ],
-      "action": "update",
+      "action": "update_id",
       "new_state": {
         "value": {
           "object_id": "",
@@ -79,7 +79,8 @@
           ]
         },
         "object_id": {
-          "action": "update",
+          "action": "update_id",
+          "reason": "id_changes",
           "old": "/jobs/123",
           "new": "",
           "remote": "/jobs/123"

acceptance/bundle/state/permission_level_migration/output.txt

@@ -34,7 +34,7 @@ Deployment complete!
    }
   },
   "resources.jobs.my_job.permissions": {
-   "__id__": "/jobs/123",
+   "__id__": "/jobs/[NUMID]",
    "state": {
     "object_id": "/jobs/[NUMID]",
     "__embed__": [

bundle/direct/apply.go

@@ -287,12 +287,12 @@ func (d *DeploymentUnit) loadPersistedState(db *dstate.DeploymentState) (any, er
 	return state, nil
 }
 
-func (d *DeploymentUnit) refreshRemoteState(ctx context.Context, id string) error {
+func (d *DeploymentUnit) refreshRemoteState(ctx context.Context, id string, newState any) error {
 	if d.RemoteState != nil {
 		return nil
 	}
 	remoteState, err := retryOnTransient(ctx, func() (any, error) {
-		return d.Adapter.DoRead(ctx, id)
+		return d.Adapter.DoRead(ctx, id, newState)
 	})
 	if err != nil {
 		return fmt.Errorf("failed to refresh remote state id=%s: %w", id, err)

bundle/direct/bundle_apply.go

@@ -106,6 +106,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
 
 		// We don't keep NewState around for 'skip' nodes
 
+		var newState any
 		if action != deployplan.Skip {
 			if !b.resolveReferences(ctx, resourceKey, entry, errorPrefix, false) {
 				return false
@@ -123,6 +124,8 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
 				return false
 			}
 
+			newState = sv.Value
+
 			if migrateMode {
 				// In migration mode we're reading resources in DAG order so that we have fully resolved config snapshots stored
 				id := b.StateDB.GetResourceID(resourceKey)
@@ -153,7 +156,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
 				return false
 			}
 
-			err = d.refreshRemoteState(ctx, id)
+			err = d.refreshRemoteState(ctx, id, newState)
 			if err != nil {
 				logdiag.LogError(ctx, fmt.Errorf("%s: failed to read remote state: %w", errorPrefix, err))
 				return false

bundle/direct/bundle_plan.go

@@ -157,8 +157,10 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
 				return false
 			}
 
+			// Delete branch: there is no planned new state, pass typed nil so the
+			// adapter falls back to using the deployment state id.
 			remoteState, err := retryOnTransient(ctx, func() (any, error) {
-				return adapter.DoRead(ctx, id)
+				return adapter.DoRead(ctx, id, nil)
 			})
 			if err != nil {
 				if isResourceGone(err) {
@@ -215,7 +217,7 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
 		}
 
 		remoteState, err := retryOnTransient(ctx, func() (any, error) {
-			return adapter.DoRead(ctx, dbentry.ID)
+			return adapter.DoRead(ctx, dbentry.ID, sv.Value)
 		})
 		if err != nil {
 			if isResourceGone(err) {

bundle/direct/dresources/adapter.go

@@ -39,8 +39,12 @@ type IResource interface {
 	RemapState(input any) any
 
 	// DoRead reads and returns remote state from the backend. The return type defines schema for remote field resolution.
-	// Example: func (r *ResourceJob) DoRead(ctx context.Context, id string) (*jobs.Job, error)
-	DoRead(ctx context.Context, id string) (remoteState any, e error)
+	// newState is the resource's planned state for the current deploy with all references resolved;
+	// resources may use it to read against an identifier computed from the new config (e.g. the
+	// permissions resource derives object_id from the parent's freshly-resolved id) instead of the
+	// possibly-stale id stored in deployment state.
+	// Example: func (r *ResourceJob) DoRead(ctx context.Context, id string, newState *jobs.JobSettings) (*jobs.Job, error)
+	DoRead(ctx context.Context, id string, newState any) (remoteState any, e error)
 
 	// DoDelete deletes the resource. The state argument is the last-persisted
 	// state for the resource; resources that don't need it should accept it as
@@ -279,6 +283,7 @@ func (a *Adapter) validate() error {
 	validations := []any{
 		"PrepareState return", a.prepareState.OutTypes[0], stateType,
 		"DoCreate newState", a.doCreate.InTypes[1], stateType,
+		"DoRead newState", a.doRefresh.InTypes[2], stateType,
 		"DoDelete state", a.doDelete.InTypes[2], stateType,
 	}
 
@@ -407,8 +412,8 @@ func (a *Adapter) RemapState(remoteState any) (any, error) {
 	return outs[0], nil
 }
 
-func (a *Adapter) DoRead(ctx context.Context, id string) (any, error) {
-	outs, err := a.doRefresh.Call(ctx, id)
+func (a *Adapter) DoRead(ctx context.Context, id string, newState any) (any, error) {
+	outs, err := a.doRefresh.Call(ctx, id, newState)
 	if err != nil {
 		return nil, err
 	}

bundle/direct/dresources/alert.go

@@ -23,7 +23,7 @@ func (*ResourceAlert) PrepareState(input *resources.Alert) *sql.AlertV2 {
 }
 
 // DoRead reads the alert by id.
-func (r *ResourceAlert) DoRead(ctx context.Context, id string) (*sql.AlertV2, error) {
+func (r *ResourceAlert) DoRead(ctx context.Context, id string, _ *sql.AlertV2) (*sql.AlertV2, error) {
 	alert, err := r.client.AlertsV2.GetAlertById(ctx, id)
 	if err != nil {
 		return nil, err

bundle/direct/dresources/all_test.go

@@ -849,8 +849,11 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
 
 	ctx := t.Context()
 
-	// initial DoRead() cannot find the resource
-	remote, err := adapter.DoRead(ctx, "1234")
+	// initial DoRead() cannot find the resource. Pass nil newState so resources that
+	// use newState (e.g. permissions) fall back to the id-based path and surface the
+	// expected "not found" error rather than reading against the test fixture's
+	// already-created parent.
+	remote, err := adapter.DoRead(ctx, "1234", nil)
 	require.Nil(t, remote)
 	require.Error(t, err)
 	// TODO: if errors.Is(err, databricks.ErrResourceDoesNotExist) {... }
@@ -859,7 +862,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
 	require.NoError(t, err, "DoCreate failed state=%v", newState)
 	require.NotEmpty(t, createdID, "ID returned from DoCreate was empty")
 
-	remote, err = adapter.DoRead(ctx, createdID)
+	remote, err = adapter.DoRead(ctx, createdID, newState)
 	require.NoError(t, err)
 	require.NotNil(t, remote)
 
@@ -944,7 +947,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
 
 	deleteIsNoop := strings.HasSuffix(group, "permissions") || strings.HasSuffix(group, "grants")
 
-	remoteAfterDelete, err := adapter.DoRead(ctx, createdID)
+	remoteAfterDelete, err := adapter.DoRead(ctx, createdID, newState)
 	if deleteIsNoop {
 		require.NoError(t, err)
 	} else {

bundle/direct/dresources/app.go

@@ -93,7 +93,7 @@ func (*ResourceApp) RemapState(remote *AppRemote) *AppState {
 	}
 }
 
-func (r *ResourceApp) DoRead(ctx context.Context, id string) (*AppRemote, error) {
+func (r *ResourceApp) DoRead(ctx context.Context, id string, _ *AppState) (*AppRemote, error) {
 	app, err := r.client.Apps.GetByName(ctx, id)
 	if err != nil {
 		return nil, err

bundle/direct/dresources/catalog.go

@@ -36,7 +36,7 @@ func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCat
 	}
 }
 
-func (r *ResourceCatalog) DoRead(ctx context.Context, id string) (*catalog.CatalogInfo, error) {
+func (r *ResourceCatalog) DoRead(ctx context.Context, id string, _ *catalog.CreateCatalog) (*catalog.CatalogInfo, error) {
 	return r.client.Catalogs.GetByName(ctx, id)
 }
 

bundle/direct/dresources/cluster.go

@@ -124,7 +124,7 @@ func (r *ResourceCluster) RemapState(input *ClusterRemote) *ClusterState {
 	return spec
 }
 
-func (r *ResourceCluster) DoRead(ctx context.Context, id string) (*ClusterRemote, error) {
+func (r *ResourceCluster) DoRead(ctx context.Context, id string, _ *ClusterState) (*ClusterRemote, error) {
 	details, err := r.client.Clusters.GetByClusterId(ctx, id)
 	if err != nil {
 		return nil, err

bundle/direct/dresources/dashboard.go

@@ -106,7 +106,7 @@ func (r *ResourceDashboard) RemapState(state *DashboardState) *DashboardState {
 	}
 }
 
-func (r *ResourceDashboard) DoRead(ctx context.Context, id string) (*DashboardState, error) {
+func (r *ResourceDashboard) DoRead(ctx context.Context, id string, _ *DashboardState) (*DashboardState, error) {
 	var dashboard *dashboards.Dashboard
 	var publishedDashboard *dashboards.PublishedDashboard
 	var publishedErr error

bundle/direct/dresources/database_catalog.go

@@ -20,7 +20,7 @@ func (*ResourceDatabaseCatalog) PrepareState(input *resources.DatabaseCatalog) *
 	return &input.DatabaseCatalog
 }
 
-func (r *ResourceDatabaseCatalog) DoRead(ctx context.Context, id string) (*database.DatabaseCatalog, error) {
+func (r *ResourceDatabaseCatalog) DoRead(ctx context.Context, id string, _ *database.DatabaseCatalog) (*database.DatabaseCatalog, error) {
 	return r.client.Database.GetDatabaseCatalogByName(ctx, id)
 }
 

bundle/direct/dresources/database_instance.go

@@ -21,7 +21,7 @@ func (*ResourceDatabaseInstance) PrepareState(input *resources.DatabaseInstance)
 	return &input.DatabaseInstance
 }
 
-func (d *ResourceDatabaseInstance) DoRead(ctx context.Context, id string) (*database.DatabaseInstance, error) {
+func (d *ResourceDatabaseInstance) DoRead(ctx context.Context, id string, _ *database.DatabaseInstance) (*database.DatabaseInstance, error) {
 	return d.client.Database.GetDatabaseInstanceByName(ctx, id)
 }
 

bundle/direct/dresources/experiment.go

@@ -40,7 +40,7 @@ func (*ResourceExperiment) RemapState(experiment *ml.Experiment) *ml.CreateExper
 	}
 }
 
-func (r *ResourceExperiment) DoRead(ctx context.Context, id string) (*ml.Experiment, error) {
+func (r *ResourceExperiment) DoRead(ctx context.Context, id string, _ *ml.CreateExperiment) (*ml.Experiment, error) {
 	result, err := r.client.Experiments.GetExperiment(ctx, ml.GetExperimentRequest{
 		ExperimentId: id,
 	})

bundle/direct/dresources/external_location.go

@@ -40,7 +40,7 @@ func (*ResourceExternalLocation) RemapState(info *catalog.ExternalLocationInfo)
 	}
 }
 
-func (r *ResourceExternalLocation) DoRead(ctx context.Context, id string) (*catalog.ExternalLocationInfo, error) {
+func (r *ResourceExternalLocation) DoRead(ctx context.Context, id string, _ *catalog.CreateExternalLocation) (*catalog.ExternalLocationInfo, error) {
 	return r.client.ExternalLocations.GetByName(ctx, id)
 }
 

bundle/direct/dresources/genie_space.go

@@ -58,7 +58,7 @@ func (r *ResourceGenieSpace) RemapState(remote *resources.GenieSpaceConfig) *res
 	}
 }
 
-func (r *ResourceGenieSpace) DoRead(ctx context.Context, id string) (*resources.GenieSpaceConfig, error) {
+func (r *ResourceGenieSpace) DoRead(ctx context.Context, id string, _ *resources.GenieSpaceConfig) (*resources.GenieSpaceConfig, error) {
 	space, err := r.client.Genie.GetSpace(ctx, dashboards.GenieGetSpaceRequest{
 		SpaceId:                id,
 		IncludeSerializedSpace: true, // otherwise etag isn't returned