Fix spurious grants create for empty grants list on direct engine

.nextchanges/bundles/empty-grants-migrate.md

@@ -0,0 +1 @@
+Fixed the direct deployment engine planning a spurious `create` for an empty `grants: []` list. Terraform records no grants resource for such a list, so `bundle plan` after `bundle deployment migrate` no longer reports an action for it. Emptying a previously deployed list still revokes the grants, after which the node is dropped from the deployment state instead of being reported as unchanged forever.

acceptance/bundle/invariant/migrate/test.toml

@@ -26,17 +26,6 @@ EnvMatrixExclude.no_cross_resource_ref = ["INPUT_CONFIG=job_cross_resource_ref.y
 # Grant cross-references require the EmbeddedSlice pattern not present in terraform mode.
 EnvMatrixExclude.no_grant_ref = ["INPUT_CONFIG=schema_grant_ref.yml.tmpl"]
 
-# An empty grants list plans a spurious create after migrate: Terraform records no
-# databricks_grants resource for grants: [], so migrate leaves no state entry for the
-# grants node, but the direct plan emits a "create" for it anyway. Found by fuzz testing.
-# The plan check fails with:
-# Unexpected action='create' for resources.schemas.foo.grants
-# ...
-# "resources.schemas.foo.grants": { "action": "create", ... }
-# Exit code: 10
-# Fixed by https://github.com/databricks/cli/pull/6039; re-enable once that lands.
-EnvMatrixExclude.no_empty_grants = ["INPUT_CONFIG=schema_empty_grants.yml.tmpl"]
-
 # SQL warehouses currently failing with migration with permanent drift. TODO: fix this.
 EnvMatrixExclude.no_sql_warehouse = ["INPUT_CONFIG=sql_warehouse.yml.tmpl"]
 

acceptance/bundle/resources/grants/schemas/remove_all/databricks.yml.tmpl

@@ -0,0 +1,9 @@
+bundle:
+ name: schema-grants-remove-all-$UNIQUE_NAME
+
+resources:
+ schemas:
+ grants_schema:
+ name: schema_remove_all_$UNIQUE_NAME
+ catalog_name: main
+ grants: [{ principal: deco-test-user@databricks.com, privileges: [USE_SCHEMA] }]

acceptance/bundle/resources/grants/schemas/remove_all/out.plan.txt

@@ -0,0 +1,12 @@
+
+>>> [CLI] bundle plan
+Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/schema-grants-remove-all-[UNIQUE_NAME]/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> [CLI] bundle plan
+Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged

acceptance/bundle/resources/grants/schemas/remove_all/output.txt

@@ -0,0 +1,31 @@
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/schema-grants-remove-all-[UNIQUE_NAME]/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> [CLI] grants get schema main.schema_remove_all_[UNIQUE_NAME]
+json.privilege_assignments[].principal = "deco-test-user@databricks.com";
+json.privilege_assignments[].privileges[] = "USE_SCHEMA";
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/schema-grants-remove-all-[UNIQUE_NAME]/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> [CLI] grants get schema main.schema_remove_all_[UNIQUE_NAME]
+json = {};
+
+>>> 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-remove-all-[UNIQUE_NAME]/default
+
+Deleting files...
+Destroy complete!

acceptance/bundle/resources/grants/schemas/remove_all/script

@@ -0,0 +1,26 @@
+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
+trace $CLI grants get schema main.schema_remove_all_$UNIQUE_NAME | gron.py --noindex | sort_lines.py --repl | contains.py 'deco-test-user@databricks.com'
+
+# Empty the whole list. The grants node has state here, so it must stay in the plan
+# and revoke; dropping it would move it to the delete branch where grants' no-op
+# DoDelete silently keeps the grant.
+update_file.py databricks.yml 'grants: [{ principal: deco-test-user@databricks.com, privileges: [USE_SCHEMA] }]' 'grants: []'
+
+trace $CLI bundle deploy
+trace $CLI grants get schema main.schema_remove_all_$UNIQUE_NAME | gron.py --noindex | sort_lines.py --repl | contains.py '!deco-test-user@databricks.com'
+
+# The revoking deploy drops the emptied grants node from state, so from here on both
+# engines plan the schema alone.
+{
+ trace $CLI bundle plan
+ trace $CLI bundle deploy
+ trace $CLI bundle plan
+} &> out.plan.txt

bundle/direct/apply.go

@@ -149,9 +149,24 @@ func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState,
  return err
 	}
 
-	err = db.SaveState(d.ResourceKey, id, newState, d.DependsOn)
+empty, err := d.Adapter.IsEmptyState(newState)
 	if err != nil {
- return fmt.Errorf("saving state id=%s: %w", id, err)
+ 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 = db.SaveState(d.ResourceKey, id, newState, d.DependsOn)
+ if err != nil {
+ return fmt.Errorf("saving state id=%s: %w", id, err)
+ }
 	}
 
 	waitRemoteState, err := retryOnTransient(ctx, func() (any, error) {

bundle/direct/bundle_plan.go

@@ -975,6 +975,18 @@ 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
+ }
+ }
+
  // Note, we're extracting references in input config but resolving them in newState.Config which is PrepareState(inputConfig)
  // This means input and state must be compatible: input can have more fields, but existing fields should not be moved
  // This means one cannot refer to fields not present in state (e.g. ${resources.jobs.foo.permissions})

bundle/direct/dresources/README.md

@@ -42,6 +42,12 @@ For resources whose create or update is asynchronous (the resource is not immedi
 
 If the API may return a slice's elements in a different order between calls (e.g., `depends_on` in job tasks, `privileges` in grants), implement `KeyedSlices` to compare elements by a natural key rather than by index. Without this, every deploy after any reordering shows phantom diffs.
 
+## Empty states: IsEmptyState
+
+If a desired state describes no resource at all (e.g. an empty grants list), implement `IsEmptyState`. The planner omits such a node instead of planning a create, and apply drops its state entry instead of persisting one.
+
+The planner only consults it for nodes without a state entry: once state exists the node stays in the plan, so emptying it still plans an update, and the update is what removes the entry.
+
 ## State backward compatibility
 
 The state struct is serialized to JSON and persisted between deploys. Backward incompatible changes will result in a drift, which depending

bundle/direct/dresources/adapter.go

@@ -56,6 +56,12 @@ type IResource interface {
 	// Example: func (r *ResourceVolume) DoCreate(ctx context.Context, newState *catalog.CreateVolumeRequestContent) (string, *catalog.VolumeInfo, error)
 	DoCreate(ctx context.Context, newState any) (id string, remoteState any, e error)
 
+	// [Optional] IsEmptyState reports that newState describes no resource at all: the planner
+	// omits the node instead of planning a create, and apply drops the state entry instead of
+	// persisting one, so both engines converge on "this node does not exist".
+	// Example: func (*ResourceGrants) IsEmptyState(state *GrantsState) bool
+	IsEmptyState(newState any) bool
+
 	// [Optional] DoUpdate updates the resource. ID must not change as a result of this operation. Returns optionally remote state.
 	// If remote state is available as part of the operation, return it; otherwise return nil.
 	// Example: func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, newState *catalog.CreateSchema, entry *PlanEntry) (*catalog.SchemaInfo, error)
@@ -103,6 +109,7 @@ type Adapter struct {
 	doCreate *calladapt.BoundCaller
 
 	// Optional:
+	isEmptyState *calladapt.BoundCaller
 	doUpdate *calladapt.BoundCaller
 	doUpdateWithID *calladapt.BoundCaller
 	waitAfterCreate *calladapt.BoundCaller
@@ -136,6 +143,7 @@ func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceC
  doRefresh: nil,
  doDelete: nil,
  doCreate: nil,
+ isEmptyState: nil,
  doUpdate: nil,
  doUpdateWithID: nil,
  doResize: nil,
@@ -205,6 +213,11 @@ func (a *Adapter) initMethods(resource any) error {
 
 	// Optional methods with varying signatures:
 
+	a.isEmptyState, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "IsEmptyState")
+	if err != nil {
+ return err
+	}
+
 	a.doUpdate, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "DoUpdate")
 	if err != nil {
  return err
@@ -313,6 +326,10 @@ func (a *Adapter) validate() error {
 	}
 	validations = append(validations, "DoCreate remoteState return", a.doCreate.OutTypes[1], remoteType)
 
+	if a.isEmptyState != nil {
+ validations = append(validations, "IsEmptyState newState", a.isEmptyState.InTypes[0], stateType)
+	}
+
 	// Validate DoUpdate: must return (remoteType, error) if implemented
 	if a.doUpdate != nil {
  validations = append(validations, "DoUpdate newState", a.doUpdate.InTypes[2], stateType)
@@ -470,6 +487,19 @@ func (a *Adapter) DoCreate(ctx context.Context, newState any) (string, any, erro
 	return id, remoteState, nil
 }
 
+// IsEmptyState reports whether newState describes no resource; false if not implemented.
+func (a *Adapter) IsEmptyState(newState any) (bool, error) {
+	if a.isEmptyState == nil {
+ return false, nil
+	}
+
+	outs, err := a.isEmptyState.Call(newState)
+	if err != nil {
+ return false, err
+	}
+	return outs[0].(bool), nil
+}
+
 // HasDoUpdate returns true if the resource implements DoUpdate method.
 func (a *Adapter) HasDoUpdate() bool {
 	return a.doUpdate != nil

bundle/direct/dresources/grants.go

@@ -76,6 +76,13 @@ func (*ResourceGrants) PrepareState(state *GrantsState) *GrantsState {
 	return state
 }
 
+// IsEmptyState reports an empty grants list as no resource at all: nothing to grant, and
+// Terraform records no databricks_grants resource for it either, so migrated bundles have
+// no state entry.
+func (*ResourceGrants) IsEmptyState(state *GrantsState) bool {
+	return len(state.EmbeddedSlice) == 0
+}
+
 func grantKey(x catalog.PrivilegeAssignment) (string, string) {
 	return "principal", x.Principal
 }

bundle/direct/dresources/grants_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestBuildGrantChanges(t *testing.T) {
@@ -75,6 +76,57 @@ func TestBuildGrantChanges(t *testing.T) {
 	}
 }
 
+// Calls through the adapter so the optional-method wiring is covered too.
+func TestGrantsIsEmptyState(t *testing.T) {
+	tests := []struct {
+ name string
+ state *GrantsState
+ expected bool
+	}{
+ {
+ name: "empty grants list",
+ state: &GrantsState{SecurableType: "schema", EmbeddedSlice: []catalog.PrivilegeAssignment{}},
+ expected: true,
+ },
+ {
+ name: "unset grants list",
+ state: &GrantsState{SecurableType: "schema"},
+ expected: true,
+ },
+ {
+ name: "one assignment",
+ state: &GrantsState{
+ SecurableType: "schema",
+ EmbeddedSlice: []catalog.PrivilegeAssignment{
+ {Principal: "alice", Privileges: []catalog.Privilege{catalog.PrivilegeSelect}},
+ },
+ },
+ expected: false,
+ },
+	}
+
+	adapter, err := NewAdapter(SupportedResources["schemas.grants"], "schemas.grants", nil)
+	require.NoError(t, err)
+
+	for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ empty, err := adapter.IsEmptyState(tt.state)
+ require.NoError(t, err)
+ assert.Equal(t, tt.expected, empty)
+ })
+	}
+}
+
+// Resources without IsEmptyState are never treated as empty.
+func TestIsEmptyStateNotImplemented(t *testing.T) {
+	adapter, err := NewAdapter(SupportedResources["schemas"], "schemas", nil)
+	require.NoError(t, err)
+
+	empty, err := adapter.IsEmptyState(&catalog.CreateSchema{Name: "myschema"})
+	require.NoError(t, err)
+	assert.False(t, empty)
+}
+
 func TestNormalizeAssignments(t *testing.T) {
 	tests := []struct {
  name string