From 23ac1f9b4edabf6cd707649648084496d06b6362 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Thu, 23 Jul 2026 12:53:39 +0000 Subject: [PATCH 1/5] Fix spurious grants create for empty grants list on direct engine An empty grants: [] list produced no databricks_grants resource in terraform state, so bundle deployment migrate wrote no grants entry. The direct engine's makePlan then always emitted a plan node for the grants config node, so bundle plan showed a spurious 'create' for the empty grants node -- contradicting migrate's 'no actions planned'. Skip an empty grants node in makePlan when there is no existing state entry. When a state entry exists (grants were deployed and are now being emptied), the node is still emitted so the revoke is planned. --- .nextchanges/bundles/empty-grants-migrate.md | 1 + bundle/direct/bundle_plan.go | 13 +++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 .nextchanges/bundles/empty-grants-migrate.md diff --git a/.nextchanges/bundles/empty-grants-migrate.md b/.nextchanges/bundles/empty-grants-migrate.md new file mode 100644 index 00000000000..121aa2b4c1f --- /dev/null +++ b/.nextchanges/bundles/empty-grants-migrate.md @@ -0,0 +1 @@ +Fixed the direct deployment engine planning a spurious `create` for a resource's empty `grants: []` list. An empty grants list with no existing state entry is now a no-op, so `bundle plan` reports no actions after `bundle deployment migrate` (Terraform never records a grants resource for an empty list). diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 139012423ea..d074be52814 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -961,6 +961,19 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root if err != nil { return nil, err } + // An empty grants list with no state entry is a no-op: there is nothing + // to create and, unlike a non-empty-to-empty edit, no principals to revoke. + // Terraform never writes a databricks_grants resource for an empty list, so + // after "bundle deployment migrate" there is no state entry to match against; + // emitting a plan node here would show a spurious "create" and contradict + // migrate's promise of no actions planned. When a state entry does exist + // (grants were deployed and are now being emptied), keep the node so the + // revoke is still planned. + if gs, ok := inputConfigStructVar.Value.(*dresources.GrantsState); ok && len(gs.EmbeddedSlice) == 0 { + if _, hasState := db.State[node]; !hasState { + continue + } + } inputConfig = inputConfigStructVar.Value baseRefs = inputConfigStructVar.Refs } From e4d8d20adcea278ee4b7b01051847c17453a1667 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Thu, 23 Jul 2026 13:01:53 +0000 Subject: [PATCH 2/5] Shorten comments in empty-grants fix --- bundle/direct/bundle_plan.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index d074be52814..07ff3257588 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -961,14 +961,9 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root if err != nil { return nil, err } - // An empty grants list with no state entry is a no-op: there is nothing - // to create and, unlike a non-empty-to-empty edit, no principals to revoke. - // Terraform never writes a databricks_grants resource for an empty list, so - // after "bundle deployment migrate" there is no state entry to match against; - // emitting a plan node here would show a spurious "create" and contradict - // migrate's promise of no actions planned. When a state entry does exist - // (grants were deployed and are now being emptied), keep the node so the - // revoke is still planned. + // Terraform writes no grants resource for an empty list, so migrate leaves + // no state entry. Skip the node here too; otherwise plan shows a spurious + // "create". Keep it when state exists so emptying grants still revokes. if gs, ok := inputConfigStructVar.Value.(*dresources.GrantsState); ok && len(gs.EmbeddedSlice) == 0 { if _, hasState := db.State[node]; !hasState { continue From 5a47a082f2145f0943eaf7ba7121f64ad9ab731a Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Fri, 24 Jul 2026 07:31:02 +0000 Subject: [PATCH 3/5] acc: re-enable empty-grants invariant migrate variant With the direct-engine fix in place, the migrate invariant no longer shows a spurious grants -> create for an empty grants: [] list, so remove the no_empty_grants exclude added by the invariant-config PR and regenerate the migrate matrix. This is the variant the fuzz-found config was disabled for. --- acceptance/bundle/invariant/migrate/out.test.toml | 1 + acceptance/bundle/invariant/migrate/test.toml | 11 ----------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/acceptance/bundle/invariant/migrate/out.test.toml b/acceptance/bundle/invariant/migrate/out.test.toml index 0ac1b789d6f..469a1c81839 100644 --- a/acceptance/bundle/invariant/migrate/out.test.toml +++ b/acceptance/bundle/invariant/migrate/out.test.toml @@ -34,6 +34,7 @@ EnvMatrix.INPUT_CONFIG = [ "postgres_synced_table.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", + "schema_empty_grants.yml.tmpl", "schema_uppercase_name.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", diff --git a/acceptance/bundle/invariant/migrate/test.toml b/acceptance/bundle/invariant/migrate/test.toml index 37dc2557922..a91f990df3b 100644 --- a/acceptance/bundle/invariant/migrate/test.toml +++ b/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"] From 661aa4e812e2bf23e507f7e7e66af30630db59db Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Mon, 27 Jul 2026 07:05:35 +0000 Subject: [PATCH 4/5] Move empty-grants skip behind an optional SkipCreate resource method The planner asserted on *dresources.GrantsState directly to detect an empty grants list. Add an optional SkipCreate method to IResource so the predicate lives with the resource, and have makePlan consult it through the adapter. The state lookup stays in the planner: a node that already has state must keep its plan entry even when creating it would be a no-op, otherwise it falls into the delete branch and grants' no-op DoDelete would silently stop revoking. --- bundle/direct/bundle_plan.go | 20 ++++++---- bundle/direct/dresources/README.md | 4 ++ bundle/direct/dresources/adapter.go | 31 +++++++++++++++ bundle/direct/dresources/grants.go | 7 ++++ bundle/direct/dresources/grants_test.go | 53 +++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 8 deletions(-) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 07ff3257588..ac62e57049f 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -961,14 +961,6 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root if err != nil { return nil, err } - // Terraform writes no grants resource for an empty list, so migrate leaves - // no state entry. Skip the node here too; otherwise plan shows a spurious - // "create". Keep it when state exists so emptying grants still revokes. - if gs, ok := inputConfigStructVar.Value.(*dresources.GrantsState); ok && len(gs.EmbeddedSlice) == 0 { - if _, hasState := db.State[node]; !hasState { - continue - } - } inputConfig = inputConfigStructVar.Value baseRefs = inputConfigStructVar.Refs } @@ -978,6 +970,18 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root return nil, fmt.Errorf("%s: %w", prefix, err) } + // A node that already has state must stay in the plan even when creating it would be + // a no-op, otherwise emptying it would plan no action at all. + if _, hasState := db.State[node]; !hasState { + skip, err := adapter.SkipCreate(newStateConfig) + if err != nil { + return nil, fmt.Errorf("%s: %w", prefix, err) + } + if skip { + 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}) diff --git a/bundle/direct/dresources/README.md b/bundle/direct/dresources/README.md index da9e9edab28..fc85b49a755 100644 --- a/bundle/direct/dresources/README.md +++ b/bundle/direct/dresources/README.md @@ -42,6 +42,10 @@ 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. +## No-op creates: SkipCreate + +If a resource can have a desired state that requires no API call at all to create (e.g. an empty grants list), implement `SkipCreate` so the planner omits the node instead of planning a create. It is only consulted when the node has no state entry yet: once state exists the node stays in the plan, so that emptying it still plans the corresponding update. + ## State backward compatibility The state struct is serialized to JSON and persisted between deploys. Backward incompatible changes will result in a drift, which depending diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index 70a9f1f5e3d..4dc17c712b5 100644 --- a/bundle/direct/dresources/adapter.go +++ b/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] SkipCreate reports whether creating newState would be a no-op, so that the + // planner can omit the node instead of planning a create. Only consulted when there is no + // state entry for the node yet. + // Example: func (*ResourceGrants) SkipCreate(state *GrantsState) bool + SkipCreate(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) @@ -96,6 +102,7 @@ type Adapter struct { doCreate *calladapt.BoundCaller // Optional: + skipCreate *calladapt.BoundCaller doUpdate *calladapt.BoundCaller doUpdateWithID *calladapt.BoundCaller waitAfterCreate *calladapt.BoundCaller @@ -128,6 +135,7 @@ func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceC doRefresh: nil, doDelete: nil, doCreate: nil, + skipCreate: nil, doUpdate: nil, doUpdateWithID: nil, doResize: nil, @@ -196,6 +204,11 @@ func (a *Adapter) initMethods(resource any) error { // Optional methods with varying signatures: + a.skipCreate, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "SkipCreate") + if err != nil { + return err + } + a.doUpdate, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "DoUpdate") if err != nil { return err @@ -299,6 +312,10 @@ func (a *Adapter) validate() error { } validations = append(validations, "DoCreate remoteState return", a.doCreate.OutTypes[1], remoteType) + if a.skipCreate != nil { + validations = append(validations, "SkipCreate newState", a.skipCreate.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) @@ -452,6 +469,20 @@ func (a *Adapter) DoCreate(ctx context.Context, newState any) (string, any, erro return id, remoteState, nil } +// SkipCreate reports whether creating newState would be a no-op. Resources that do not +// implement it always need a create. +func (a *Adapter) SkipCreate(newState any) (bool, error) { + if a.skipCreate == nil { + return false, nil + } + + outs, err := a.skipCreate.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 diff --git a/bundle/direct/dresources/grants.go b/bundle/direct/dresources/grants.go index bac70bc5615..f53b1ab7730 100644 --- a/bundle/direct/dresources/grants.go +++ b/bundle/direct/dresources/grants.go @@ -77,6 +77,13 @@ func (*ResourceGrants) PrepareState(state *GrantsState) *GrantsState { return state } +// SkipCreate reports that an empty grants list needs no create: there is nothing to grant, +// and Terraform records no databricks_grants resource for it either, so a bundle migrated +// from Terraform has no state entry for the node. +func (*ResourceGrants) SkipCreate(state *GrantsState) bool { + return len(state.EmbeddedSlice) == 0 +} + func grantKey(x catalog.PrivilegeAssignment) (string, string) { return "principal", x.Principal } diff --git a/bundle/direct/dresources/grants_test.go b/bundle/direct/dresources/grants_test.go index 1724eed40dc..e6ef39a165e 100644 --- a/bundle/direct/dresources/grants_test.go +++ b/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) { @@ -74,3 +75,55 @@ func TestBuildGrantChanges(t *testing.T) { }) } } + +// Goes through the adapter rather than calling the method directly, so that the optional +// method is also checked to be discovered and type-validated by NewAdapter. +func TestGrantsSkipCreate(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) { + skip, err := adapter.SkipCreate(tt.state) + require.NoError(t, err) + assert.Equal(t, tt.expected, skip) + }) + } +} + +// Resources without SkipCreate always need a create. +func TestSkipCreateNotImplemented(t *testing.T) { + adapter, err := NewAdapter(SupportedResources["schemas"], "schemas", nil) + require.NoError(t, err) + + skip, err := adapter.SkipCreate(&catalog.CreateSchema{Name: "myschema"}) + require.NoError(t, err) + assert.False(t, skip) +} From d934ef9474a80aa0e687c3011a307e323e51d8b1 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Mon, 27 Jul 2026 07:11:43 +0000 Subject: [PATCH 5/5] Shorten comments --- .nextchanges/bundles/empty-grants-migrate.md | 2 +- bundle/direct/bundle_plan.go | 3 +-- bundle/direct/dresources/README.md | 2 +- bundle/direct/dresources/adapter.go | 8 +++----- bundle/direct/dresources/grants.go | 5 ++--- bundle/direct/dresources/grants_test.go | 3 +-- 6 files changed, 9 insertions(+), 14 deletions(-) diff --git a/.nextchanges/bundles/empty-grants-migrate.md b/.nextchanges/bundles/empty-grants-migrate.md index 121aa2b4c1f..01b1434d007 100644 --- a/.nextchanges/bundles/empty-grants-migrate.md +++ b/.nextchanges/bundles/empty-grants-migrate.md @@ -1 +1 @@ -Fixed the direct deployment engine planning a spurious `create` for a resource's empty `grants: []` list. An empty grants list with no existing state entry is now a no-op, so `bundle plan` reports no actions after `bundle deployment migrate` (Terraform never records a grants resource for an empty list). +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. diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index ac62e57049f..6756c41dd21 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -970,8 +970,7 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root return nil, fmt.Errorf("%s: %w", prefix, err) } - // A node that already has state must stay in the plan even when creating it would be - // a no-op, otherwise emptying it would plan no action at all. + // New nodes only: a node with state must stay in the plan, otherwise emptying it plans nothing. if _, hasState := db.State[node]; !hasState { skip, err := adapter.SkipCreate(newStateConfig) if err != nil { diff --git a/bundle/direct/dresources/README.md b/bundle/direct/dresources/README.md index fc85b49a755..6ff3c7518e9 100644 --- a/bundle/direct/dresources/README.md +++ b/bundle/direct/dresources/README.md @@ -44,7 +44,7 @@ If the API may return a slice's elements in a different order between calls (e.g ## No-op creates: SkipCreate -If a resource can have a desired state that requires no API call at all to create (e.g. an empty grants list), implement `SkipCreate` so the planner omits the node instead of planning a create. It is only consulted when the node has no state entry yet: once state exists the node stays in the plan, so that emptying it still plans the corresponding update. +If a desired state requires no API call to create (e.g. an empty grants list), implement `SkipCreate` and the planner omits the node instead of planning a create. It is only consulted for nodes without a state entry: once state exists the node stays in the plan, so emptying it still plans an update. ## State backward compatibility diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index 4dc17c712b5..f3df51b42be 100644 --- a/bundle/direct/dresources/adapter.go +++ b/bundle/direct/dresources/adapter.go @@ -56,9 +56,8 @@ 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] SkipCreate reports whether creating newState would be a no-op, so that the - // planner can omit the node instead of planning a create. Only consulted when there is no - // state entry for the node yet. + // [Optional] SkipCreate reports that creating newState is a no-op, so the planner omits the + // node instead of planning a create. Only consulted for nodes without a state entry. // Example: func (*ResourceGrants) SkipCreate(state *GrantsState) bool SkipCreate(newState any) bool @@ -469,8 +468,7 @@ func (a *Adapter) DoCreate(ctx context.Context, newState any) (string, any, erro return id, remoteState, nil } -// SkipCreate reports whether creating newState would be a no-op. Resources that do not -// implement it always need a create. +// SkipCreate reports whether creating newState is a no-op; false if not implemented. func (a *Adapter) SkipCreate(newState any) (bool, error) { if a.skipCreate == nil { return false, nil diff --git a/bundle/direct/dresources/grants.go b/bundle/direct/dresources/grants.go index f53b1ab7730..67c5db060c0 100644 --- a/bundle/direct/dresources/grants.go +++ b/bundle/direct/dresources/grants.go @@ -77,9 +77,8 @@ func (*ResourceGrants) PrepareState(state *GrantsState) *GrantsState { return state } -// SkipCreate reports that an empty grants list needs no create: there is nothing to grant, -// and Terraform records no databricks_grants resource for it either, so a bundle migrated -// from Terraform has no state entry for the node. +// SkipCreate reports an empty grants list as a no-op: nothing to grant, and Terraform +// records no databricks_grants resource for it, so migrated bundles have no state entry. func (*ResourceGrants) SkipCreate(state *GrantsState) bool { return len(state.EmbeddedSlice) == 0 } diff --git a/bundle/direct/dresources/grants_test.go b/bundle/direct/dresources/grants_test.go index e6ef39a165e..4b0eee206f7 100644 --- a/bundle/direct/dresources/grants_test.go +++ b/bundle/direct/dresources/grants_test.go @@ -76,8 +76,7 @@ func TestBuildGrantChanges(t *testing.T) { } } -// Goes through the adapter rather than calling the method directly, so that the optional -// method is also checked to be discovered and type-validated by NewAdapter. +// Calls through the adapter so the optional-method wiring is covered too. func TestGrantsSkipCreate(t *testing.T) { tests := []struct { name string