From 72e8f842122fd6bb642c32656e31b622eee5f562 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 23:08:21 +0200 Subject: [PATCH 1/2] Make experimental YAML-sync state upload best-effort The UploadStateForYamlSync mutator runs after a successful deploy when DATABRICKS_BUNDLE_ENABLE_EXPERIMENTAL_YAML_SYNC is set. Its state conversion reuses direct-engine code (SecretScopeFixups, CalculatePlan, Apply) that reports failures via logdiag on the shared deploy context, flipping the whole deploy to exit 1. A schema with an empty grants list triggers this: terraform emits no grants resource for an empty list, so the migration plan has a node with no state entry. Collect the conversion's diagnostics in an isolated logdiag scope and downgrade them to warnings, consistent with the mutator's existing best-effort error handling. Skip uploading the snapshot when conversion failed so a partial snapshot is never published. Co-authored-by: Isaac --- .../yaml-sync-empty-grants/databricks.yml | 9 +++++++ .../yaml-sync-empty-grants/out.test.toml | 3 +++ .../deploy/yaml-sync-empty-grants/output.txt | 8 ++++++ .../deploy/yaml-sync-empty-grants/script | 1 + .../deploy/yaml-sync-empty-grants/test.toml | 8 ++++++ .../statemgmt/upload_state_for_yaml_sync.go | 22 ++++++++++++++++ libs/logdiag/logdiag.go | 8 ++++++ libs/logdiag/logdiag_test.go | 25 +++++++++++++++++++ 8 files changed, 84 insertions(+) create mode 100644 acceptance/bundle/deploy/yaml-sync-empty-grants/databricks.yml create mode 100644 acceptance/bundle/deploy/yaml-sync-empty-grants/out.test.toml create mode 100644 acceptance/bundle/deploy/yaml-sync-empty-grants/output.txt create mode 100644 acceptance/bundle/deploy/yaml-sync-empty-grants/script create mode 100644 acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml create mode 100644 libs/logdiag/logdiag_test.go diff --git a/acceptance/bundle/deploy/yaml-sync-empty-grants/databricks.yml b/acceptance/bundle/deploy/yaml-sync-empty-grants/databricks.yml new file mode 100644 index 00000000000..87958c8e630 --- /dev/null +++ b/acceptance/bundle/deploy/yaml-sync-empty-grants/databricks.yml @@ -0,0 +1,9 @@ +bundle: + name: yaml-sync-empty-grants + +resources: + schemas: + schema1: + name: myschema + catalog_name: main + grants: [] diff --git a/acceptance/bundle/deploy/yaml-sync-empty-grants/out.test.toml b/acceptance/bundle/deploy/yaml-sync-empty-grants/out.test.toml new file mode 100644 index 00000000000..65156e0457c --- /dev/null +++ b/acceptance/bundle/deploy/yaml-sync-empty-grants/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform"] diff --git a/acceptance/bundle/deploy/yaml-sync-empty-grants/output.txt b/acceptance/bundle/deploy/yaml-sync-empty-grants/output.txt new file mode 100644 index 00000000000..fa2643665d8 --- /dev/null +++ b/acceptance/bundle/deploy/yaml-sync-empty-grants/output.txt @@ -0,0 +1,8 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/yaml-sync-empty-grants/default/files... +Deploying resources... +Updating deployment state... +Warn: Failed to create config snapshot: state conversion failed +Warn: Config snapshot: state entry not found for "resources.schemas.schema1.grants" +Deployment complete! diff --git a/acceptance/bundle/deploy/yaml-sync-empty-grants/script b/acceptance/bundle/deploy/yaml-sync-empty-grants/script new file mode 100644 index 00000000000..68ebb78d775 --- /dev/null +++ b/acceptance/bundle/deploy/yaml-sync-empty-grants/script @@ -0,0 +1 @@ +trace $CLI bundle deploy diff --git a/acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml b/acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml new file mode 100644 index 00000000000..b35a21445ee --- /dev/null +++ b/acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml @@ -0,0 +1,8 @@ +# The YAML-sync state upload only runs for the terraform engine +# (it converts terraform state to the direct format), so there is +# no direct-engine variant of this test. +[Env] +DATABRICKS_BUNDLE_ENABLE_EXPERIMENTAL_YAML_SYNC = "true" + +[EnvMatrix] +DATABRICKS_BUNDLE_ENGINE = ["terraform"] diff --git a/bundle/statemgmt/upload_state_for_yaml_sync.go b/bundle/statemgmt/upload_state_for_yaml_sync.go index 0399c7b31ff..cd6756d0554 100644 --- a/bundle/statemgmt/upload_state_for_yaml_sync.go +++ b/bundle/statemgmt/upload_state_for_yaml_sync.go @@ -54,6 +54,22 @@ func (m *uploadStateForYamlSync) Apply(ctx context.Context, b *bundle.Bundle) di return nil } + // convertState reuses direct-engine code (SecretScopeFixups, CalculatePlan, Apply) + // that reports failures via logdiag on the context. This mutator is best-effort and + // must not fail a deploy that already succeeded, so collect those diagnostics in an + // isolated scope and downgrade them to warnings. + ctx = logdiag.IsolatedContext(ctx) + logdiag.SetCollect(ctx, true) + defer func() { + for _, d := range logdiag.FlushCollected(ctx) { + msg := d.Summary + if d.Detail != "" { + msg += ": " + d.Detail + } + log.Warnf(ctx, "Config snapshot: %s", msg) + } + }() + _, snapshotPath := b.StateFilenameConfigSnapshot(ctx) created, err := m.convertState(ctx, b, snapshotPath) @@ -202,6 +218,12 @@ func (m *uploadStateForYamlSync) convertState(ctx context.Context, b *bundle.Bun return false, err } + // Apply reports failures via logdiag instead of returning an error. Don't + // upload a snapshot that is missing entries for the failed resources. + if logdiag.HasError(ctx) { + return false, errors.New("state conversion failed") + } + return true, nil } diff --git a/libs/logdiag/logdiag.go b/libs/logdiag/logdiag.go index 896d105751b..d5f4b74099f 100644 --- a/libs/logdiag/logdiag.go +++ b/libs/logdiag/logdiag.go @@ -48,6 +48,14 @@ func InitContext(ctx context.Context) context.Context { if ok { panic("internal error: must not call InitContext() twice") } + return IsolatedContext(ctx) +} + +// IsolatedContext returns a child context with a fresh diagnostics state that +// shadows the parent's. Diagnostics logged through the returned context do not +// affect the parent's counters or output. Use it for best-effort operations +// that reuse code reporting through logdiag but must not fail the command. +func IsolatedContext(ctx context.Context) context.Context { val := LogDiagData{ TargetSeverity: 255, mu: &sync.Mutex{}, diff --git a/libs/logdiag/logdiag_test.go b/libs/logdiag/logdiag_test.go new file mode 100644 index 00000000000..c4c847ddc74 --- /dev/null +++ b/libs/logdiag/logdiag_test.go @@ -0,0 +1,25 @@ +package logdiag_test + +import ( + "errors" + "testing" + + "github.com/databricks/cli/libs/logdiag" + "github.com/stretchr/testify/assert" +) + +func TestIsolatedContext(t *testing.T) { + ctx := logdiag.InitContext(t.Context()) + logdiag.SetCollect(ctx, true) + + isolated := logdiag.IsolatedContext(ctx) + logdiag.SetCollect(isolated, true) + logdiag.LogError(isolated, errors.New("inner failure")) + + assert.True(t, logdiag.HasError(isolated)) + assert.Len(t, logdiag.FlushCollected(isolated), 1) + + // Diagnostics logged through the isolated scope must not leak to the parent. + assert.False(t, logdiag.HasError(ctx)) + assert.Empty(t, logdiag.FlushCollected(ctx)) +} From d5668dd795bd00062ccb9dd2b8a266a0c2e5edc6 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Wed, 10 Jun 2026 07:24:44 +0200 Subject: [PATCH 2/2] Remove redundant comments Co-authored-by: Isaac --- acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml | 4 +--- bundle/statemgmt/upload_state_for_yaml_sync.go | 6 ++---- libs/logdiag/logdiag.go | 6 ++---- libs/logdiag/logdiag_test.go | 1 - 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml b/acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml index b35a21445ee..5799c23b638 100644 --- a/acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml +++ b/acceptance/bundle/deploy/yaml-sync-empty-grants/test.toml @@ -1,6 +1,4 @@ -# The YAML-sync state upload only runs for the terraform engine -# (it converts terraform state to the direct format), so there is -# no direct-engine variant of this test. +# The YAML-sync state upload only runs for the terraform engine; no direct-engine variant. [Env] DATABRICKS_BUNDLE_ENABLE_EXPERIMENTAL_YAML_SYNC = "true" diff --git a/bundle/statemgmt/upload_state_for_yaml_sync.go b/bundle/statemgmt/upload_state_for_yaml_sync.go index cd6756d0554..6573d1dc56f 100644 --- a/bundle/statemgmt/upload_state_for_yaml_sync.go +++ b/bundle/statemgmt/upload_state_for_yaml_sync.go @@ -54,10 +54,8 @@ func (m *uploadStateForYamlSync) Apply(ctx context.Context, b *bundle.Bundle) di return nil } - // convertState reuses direct-engine code (SecretScopeFixups, CalculatePlan, Apply) - // that reports failures via logdiag on the context. This mutator is best-effort and - // must not fail a deploy that already succeeded, so collect those diagnostics in an - // isolated scope and downgrade them to warnings. + // convertState reuses direct-engine code that reports failures via logdiag, + // and this mutator must not fail a deploy that already succeeded. ctx = logdiag.IsolatedContext(ctx) logdiag.SetCollect(ctx, true) defer func() { diff --git a/libs/logdiag/logdiag.go b/libs/logdiag/logdiag.go index d5f4b74099f..28ed3b5ba21 100644 --- a/libs/logdiag/logdiag.go +++ b/libs/logdiag/logdiag.go @@ -51,10 +51,8 @@ func InitContext(ctx context.Context) context.Context { return IsolatedContext(ctx) } -// IsolatedContext returns a child context with a fresh diagnostics state that -// shadows the parent's. Diagnostics logged through the returned context do not -// affect the parent's counters or output. Use it for best-effort operations -// that reuse code reporting through logdiag but must not fail the command. +// IsolatedContext returns a child context with a fresh diagnostics state; +// diagnostics logged through it do not affect the parent's. func IsolatedContext(ctx context.Context) context.Context { val := LogDiagData{ TargetSeverity: 255, diff --git a/libs/logdiag/logdiag_test.go b/libs/logdiag/logdiag_test.go index c4c847ddc74..ecf12ee78d1 100644 --- a/libs/logdiag/logdiag_test.go +++ b/libs/logdiag/logdiag_test.go @@ -19,7 +19,6 @@ func TestIsolatedContext(t *testing.T) { assert.True(t, logdiag.HasError(isolated)) assert.Len(t, logdiag.FlushCollected(isolated), 1) - // Diagnostics logged through the isolated scope must not leak to the parent. assert.False(t, logdiag.HasError(ctx)) assert.Empty(t, logdiag.FlushCollected(ctx)) }