diff --git a/acceptance/bundle/resources/schemas/drift/managed_properties/databricks.yml b/acceptance/bundle/resources/schemas/drift/managed_properties/databricks.yml new file mode 100644 index 00000000000..5ab0d85ca59 --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/managed_properties/databricks.yml @@ -0,0 +1,8 @@ +bundle: + name: test-bundle + +resources: + schemas: + schema1: + name: schema_managed_defaults + catalog_name: main diff --git a/acceptance/bundle/resources/schemas/drift/managed_properties/out.test.toml b/acceptance/bundle/resources/schemas/drift/managed_properties/out.test.toml new file mode 100644 index 00000000000..e90b6d5d1ba --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/managed_properties/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/schemas/drift/managed_properties/output.txt b/acceptance/bundle/resources/schemas/drift/managed_properties/output.txt new file mode 100644 index 00000000000..f2ecb3506f0 --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/managed_properties/output.txt @@ -0,0 +1,24 @@ + +=== Initial deployment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +=== Plan is a no-op despite UC auto-populating managed properties +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + +=== Redeploy is a no-op (no UpdateSchema call) +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> print_requests.py //unity +json.method = "POST"; +json.path = "/api/2.1/unity-catalog/schemas"; +json.body.catalog_name = "main"; +json.body.name = "schema_managed_defaults"; diff --git a/acceptance/bundle/resources/schemas/drift/managed_properties/script b/acceptance/bundle/resources/schemas/drift/managed_properties/script new file mode 100644 index 00000000000..c3ef4ac39e4 --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/managed_properties/script @@ -0,0 +1,11 @@ +echo "*" > .gitignore + +title "Initial deployment" +trace $CLI bundle deploy + +title "Plan is a no-op despite UC auto-populating managed properties" +trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged" + +title "Redeploy is a no-op (no UpdateSchema call)" +trace $CLI bundle deploy +trace print_requests.py //unity | gron.py | contains.py '!json.method = "PATCH"' diff --git a/acceptance/bundle/resources/schemas/drift/managed_properties/test.toml b/acceptance/bundle/resources/schemas/drift/managed_properties/test.toml new file mode 100644 index 00000000000..5016e85b395 --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/managed_properties/test.toml @@ -0,0 +1,5 @@ +RecordRequests = true + +# Terraform issues a spurious PATCH for enable_predictive_optimization on every +# deploy, which is outside the scope of backend-default handling in resources.yml. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 5591626cd75..d389cb931c9 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -454,18 +454,62 @@ func shouldSkipBackendDefault(cfg *dresources.ResourceLifecycleConfig, path *str if cfg == nil || ch.Old != nil || ch.New != nil || ch.Remote == nil { return "", false } + if matchesAnyBackendDefault(cfg, path, ch.Remote) || matchesBackendDefaultMap(cfg, path, ch.Remote) { + return deployplan.ReasonBackendDefault, true + } + return "", false +} + +// matchesAnyBackendDefault reports whether the change at path matches any of the +// resource's configured backend-default rules. +func matchesAnyBackendDefault(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNode, remote any) bool { for _, rule := range cfg.BackendDefaults { - if !path.HasPatternPrefix(rule.Field) { - continue - } - if len(rule.Values) == 0 { - return deployplan.ReasonBackendDefault, true + if matchesBackendDefaultRule(path, remote, rule) { + return true } - if matchesAllowedValue(ch.Remote, rule.Values) { - return deployplan.ReasonBackendDefault, true + } + return false +} + +func matchesBackendDefaultRule(path *structpath.PathNode, remote any, rule dresources.BackendDefaultRule) bool { + if !path.HasPatternPrefix(rule.Field) { + return false + } + if len(rule.Values) == 0 { + return true + } + return matchesAllowedValue(remote, rule.Values) +} + +// matchesBackendDefaultMap handles the nil-vs-map case from structdiff, where a +// remote-only map change is emitted at the parent path rather than per key. +// We only skip the parent map if every remote entry matches a configured +// backend-default child rule; any unmanaged key must still surface as drift. +func matchesBackendDefaultMap(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNode, remote any) bool { + rv, ok := asNonEmptyStringMap(remote) + if !ok { + return false + } + + iter := rv.MapRange() + for iter.Next() { + childPath := structpath.NewBracketString(path, iter.Key().String()) + if !matchesAnyBackendDefault(cfg, childPath, iter.Value().Interface()) { + return false } } - return "", false + + return true +} + +// asNonEmptyStringMap returns remote as a reflected map value when it is a +// non-nil, non-empty map with string keys; ok is false otherwise. +func asNonEmptyStringMap(remote any) (reflect.Value, bool) { + rv := reflect.ValueOf(remote) + if !rv.IsValid() || rv.Kind() != reflect.Map || rv.IsNil() || rv.Type().Key().Kind() != reflect.String || rv.Len() == 0 { + return reflect.Value{}, false + } + return rv, true } // matchesAllowedValue checks if the remote value matches one of the allowed JSON values. diff --git a/bundle/direct/bundle_plan_test.go b/bundle/direct/bundle_plan_test.go index ccfb7cb517f..0b3191aca30 100644 --- a/bundle/direct/bundle_plan_test.go +++ b/bundle/direct/bundle_plan_test.go @@ -3,8 +3,12 @@ package direct import ( "testing" + "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/structs/structpath" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDynPathToStructPath(t *testing.T) { @@ -35,3 +39,86 @@ func TestDynPathToStructPath(t *testing.T) { assert.Equal(t, tc.expected, node.String()) } } + +func TestShouldSkipBackendDefault_SchemaManagedPropertiesOnly(t *testing.T) { + cfg := dresources.GetResourceConfig("schemas") + require.NotNil(t, cfg) + + tests := []struct { + name string + path string + remote any + expected bool + }{ + { + name: "managed delta row tracking property", + path: "properties['unity.catalog.managed.delta.defaults.delta.enableRowTracking']", + remote: "true", + expected: true, + }, + { + name: "managed iceberg catalog property", + path: "properties['unity.catalog.managed.iceberg.defaults.delta.feature.catalogManaged']", + remote: "true", + expected: true, + }, + { + name: "unmanaged remote-only property is not skipped", + path: "properties['custom.remote_only']", + remote: "true", + expected: false, + }, + { + name: "managed-only parent properties map is skipped", + path: "properties", + remote: map[string]string{"unity.catalog.managed.delta.defaults.delta.enableRowTracking": "true"}, + expected: true, + }, + { + name: "mixed parent properties map is not skipped", + path: "properties", + remote: map[string]string{"unity.catalog.managed.delta.defaults.delta.enableRowTracking": "true", "custom.remote_only": "true"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path, err := structpath.ParsePath(tt.path) + require.NoError(t, err) + + reason, ok := shouldSkipBackendDefault(cfg, path, &deployplan.ChangeDesc{ + Old: nil, + New: nil, + Remote: tt.remote, + }) + + assert.Equal(t, tt.expected, ok) + if tt.expected { + assert.Equal(t, deployplan.ReasonBackendDefault, reason) + } else { + assert.Empty(t, reason) + } + }) + } +} + +// Map drift handling synthesizes child paths to match against rules. structdiff +// always emits map keys in bracket notation, so synthetic child paths must too; +// otherwise rules wouldn't match for identifier-like keys. +func TestShouldSkipBackendDefault_MapDriftUsesBracketKeys(t *testing.T) { + field, err := structpath.ParsePattern("properties['simple']") + require.NoError(t, err) + cfg := &dresources.ResourceLifecycleConfig{ + BackendDefaults: []dresources.BackendDefaultRule{{Field: field}}, + } + + path, err := structpath.ParsePath("properties") + require.NoError(t, err) + + reason, ok := shouldSkipBackendDefault(cfg, path, &deployplan.ChangeDesc{ + Remote: map[string]string{"simple": "v"}, + }) + assert.True(t, ok) + assert.Equal(t, deployplan.ReasonBackendDefault, reason) +} diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index 75a60e0dc62..849d3d94f32 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -311,6 +311,12 @@ resources: reason: immutable - field: storage_root reason: immutable + backend_defaults: + # UC auto-populates these system-managed keys after create. + # Without this, every subsequent plan produces an Update whose payload is empty, + # and UC rejects it with "UpdateSchema Nothing to update". + - field: properties['unity.catalog.managed.delta.defaults.delta.enableRowTracking'] + - field: properties['unity.catalog.managed.iceberg.defaults.delta.feature.catalogManaged'] external_locations: recreate_on_changes: diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index 1d4dc79e7ac..883133b8d38 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -11,6 +11,12 @@ import ( const testMetastoreName = "deco-uc-prod-isolated-aws-us-east-1" +// schemaNameManagedDefaults is the schema name the backend-default drift test uses +// to opt into UC's managed-property simulation. Scoping the injection to this name +// keeps unrelated schema tests free of the property, which terraform would otherwise +// report as drift on redeploy. +const schemaNameManagedDefaults = "schema_managed_defaults" + func (s *FakeWorkspace) SchemasCreate(req Request) Response { defer s.LockUnlock()() @@ -39,6 +45,13 @@ func (s *FakeWorkspace) SchemasCreate(req Request) Response { schema.MetastoreId = TestMetastore.MetastoreId schema.Owner = s.CurrentUser().UserName schema.SchemaId = nextUUID() + if schema.Properties == nil && schema.Name == schemaNameManagedDefaults { + // Mirror UC behavior: managed system defaults are populated when the user + // doesn't specify any properties. Required to cover backend-default drift. + schema.Properties = map[string]string{ + "unity.catalog.managed.delta.defaults.delta.enableRowTracking": "true", + } + } s.Schemas[schema.FullName] = schema return Response{