Skip to content

Commit eae333b

Browse files
direct: ignore UC-managed schema property defaults (#5195)
## Changes - Add `backend_defaults` for two fields `properties['...']` - Handle the nil-vs-map case from `structdiff` where a remote-only map change is emitted at the parent path: only skip the parent if every remote entry matches a configured child rule. Synthesize child paths with `structpath.NewBracketString` to match what `structdiff` produces (rather than `NewStringKey`, which would diverge for identifier-like keys). - Acceptance test with test server setting the properties based on schema name ## Why UC auto-populates system-managed property keys out of band (e.g. setting `unity.catalog.managed.delta.defaults.delta.enableRowTracking`). This causes the planner to output Update when there is nothing to do ## Tests - [x] Existing & new Acceptance tests - [ ] ⚠️ our current cloud test accounts do not exhibit this behaviour, only saw it in my own staging workspace This pull request and its description were written by Isaac.
1 parent 2f28778 commit eae333b

10 files changed

Lines changed: 217 additions & 4 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* Remove hidden, never-functional `--existing-dashboard-id`, `--existing-dashboard-path`, `--existing-alert-id`, and `--existing-genie-space-id` alias flags from `bundle generate`; use the documented `--existing-id` / `--existing-path` flags instead ([#5591](https://github.com/databricks/cli/pull/5591)).
1818
* engine/direct: Fix WAL corruption after two consecutive failed deploys ([#5606](https://github.com/databricks/cli/pull/5606)).
1919
* engine/direct: Don't open the deployment state WAL when a deploy's plan fails ([#5607](https://github.com/databricks/cli/pull/5607)).
20+
* Ignore unity catalog managed schema property defaults to avoid unnecessary drift ([#5195](https://github.com/databricks/cli/pull/5195)).
2021

2122
### Dependency updates
2223

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
bundle:
2+
name: test-bundle
3+
4+
resources:
5+
schemas:
6+
schema1:
7+
name: schema_managed_defaults
8+
catalog_name: main

acceptance/bundle/resources/schemas/drift/managed_properties/out.test.toml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
2+
=== Initial deployment
3+
>>> [CLI] bundle deploy
4+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
5+
Deploying resources...
6+
Updating deployment state...
7+
Deployment complete!
8+
9+
=== Plan is a no-op despite UC auto-populating managed properties
10+
>>> [CLI] bundle plan
11+
Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
12+
13+
=== The remote-only properties map is skipped as a backend default (confirms the matched rule)
14+
>>> [CLI] bundle plan --output json
15+
{
16+
"properties": {
17+
"action": "skip",
18+
"reason": "backend_default",
19+
"remote": {
20+
"unity.catalog.managed.delta.defaults.delta.enableRowTracking": "true",
21+
"unity.catalog.managed.iceberg.defaults.delta.feature.catalogManaged": "true"
22+
}
23+
}
24+
}
25+
26+
=== Redeploy is a no-op (no UpdateSchema call)
27+
>>> [CLI] bundle deploy
28+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
29+
Deploying resources...
30+
Updating deployment state...
31+
Deployment complete!
32+
33+
>>> print_requests.py //unity
34+
json.method = "POST";
35+
json.path = "/api/2.1/unity-catalog/schemas";
36+
json.body.catalog_name = "main";
37+
json.body.name = "schema_managed_defaults";
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
echo "*" > .gitignore
2+
3+
title "Initial deployment"
4+
trace $CLI bundle deploy
5+
6+
title "Plan is a no-op despite UC auto-populating managed properties"
7+
trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged"
8+
9+
title "The remote-only properties map is skipped as a backend default (confirms the matched rule)"
10+
trace $CLI bundle plan --output json | jq '.plan[].changes'
11+
12+
title "Redeploy is a no-op (no UpdateSchema call)"
13+
trace $CLI bundle deploy
14+
trace print_requests.py //unity | gron.py | contains.py '!json.method = "PATCH"'
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
RecordRequests = true
2+
3+
# Terraform issues a spurious PATCH for enable_predictive_optimization on every
4+
# deploy, which is outside the scope of backend-default handling in resources.yml.
5+
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]

bundle/direct/bundle_plan.go

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -483,18 +483,45 @@ func shouldSkipBackendDefault(cfg *dresources.ResourceLifecycleConfig, path *str
483483
if cfg == nil || ch.Old != nil || ch.New != nil || ch.Remote == nil {
484484
return "", false
485485
}
486+
if matchesAnyBackendDefault(cfg, path, ch.Remote) {
487+
return deployplan.ReasonBackendDefault, true
488+
}
489+
490+
// Nil-vs-map case from structdiff: a remote-only map change is emitted at the
491+
// parent path rather than per key. Only skip the parent map if every remote
492+
// entry matches a configured backend-default child rule; any unmanaged key
493+
// must still surface as drift. rv is always valid here (ch.Remote != nil
494+
// above) and a nil map is excluded by Len() == 0.
495+
rv := reflect.ValueOf(ch.Remote)
496+
if rv.Kind() != reflect.Map || rv.Type().Key().Kind() != reflect.String || rv.Len() == 0 {
497+
return "", false
498+
}
499+
iter := rv.MapRange()
500+
for iter.Next() {
501+
childPath := structpath.NewBracketString(path, iter.Key().String())
502+
if !matchesAnyBackendDefault(cfg, childPath, iter.Value().Interface()) {
503+
return "", false
504+
}
505+
}
506+
return deployplan.ReasonBackendDefault, true
507+
}
508+
509+
// matchesAnyBackendDefault reports whether the remote value at path matches any of
510+
// the resource's configured backend-default rules (and the rule's allowed values,
511+
// if specified).
512+
func matchesAnyBackendDefault(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNode, remote any) bool {
486513
for _, rule := range cfg.BackendDefaults {
487514
if !path.HasPatternPrefix(rule.Field) {
488515
continue
489516
}
490517
if len(rule.Values) == 0 {
491-
return deployplan.ReasonBackendDefault, true
518+
return true
492519
}
493-
if matchesAllowedValue(ch.Remote, rule.Values) {
494-
return deployplan.ReasonBackendDefault, true
520+
if matchesAllowedValue(remote, rule.Values) {
521+
return true
495522
}
496523
}
497-
return "", false
524+
return false
498525
}
499526

500527
// matchesAllowedValue checks if the remote value matches one of the allowed JSON values.

bundle/direct/bundle_plan_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ package direct
33
import (
44
"testing"
55

6+
"github.com/databricks/cli/bundle/deployplan"
7+
"github.com/databricks/cli/bundle/direct/dresources"
68
"github.com/databricks/cli/libs/dyn"
9+
"github.com/databricks/cli/libs/structs/structpath"
710
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
812
)
913

1014
func TestDynPathToStructPath(t *testing.T) {
@@ -35,3 +39,97 @@ func TestDynPathToStructPath(t *testing.T) {
3539
assert.Equal(t, tc.expected, node.String())
3640
}
3741
}
42+
43+
func TestShouldSkipBackendDefault_ManagedPropertiesOnly(t *testing.T) {
44+
// Rules mirror the schemas backend_defaults in resources.yml, but the test is
45+
// deliberately self-contained so that edits to resources.yml don't break it.
46+
// The real wiring is covered by acceptance/bundle/resources/schemas/drift.
47+
rowTracking, err := structpath.ParsePattern("properties['unity.catalog.managed.delta.defaults.delta.enableRowTracking']")
48+
require.NoError(t, err)
49+
catalogManaged, err := structpath.ParsePattern("properties['unity.catalog.managed.iceberg.defaults.delta.feature.catalogManaged']")
50+
require.NoError(t, err)
51+
cfg := &dresources.ResourceLifecycleConfig{
52+
BackendDefaults: []dresources.BackendDefaultRule{
53+
{Field: rowTracking},
54+
{Field: catalogManaged},
55+
},
56+
}
57+
58+
tests := []struct {
59+
name string
60+
path string
61+
remote any
62+
expected bool
63+
}{
64+
{
65+
name: "managed delta row tracking property",
66+
path: "properties['unity.catalog.managed.delta.defaults.delta.enableRowTracking']",
67+
remote: "true",
68+
expected: true,
69+
},
70+
{
71+
name: "managed iceberg catalog property",
72+
path: "properties['unity.catalog.managed.iceberg.defaults.delta.feature.catalogManaged']",
73+
remote: "true",
74+
expected: true,
75+
},
76+
{
77+
name: "unmanaged remote-only property is not skipped",
78+
path: "properties['custom.remote_only']",
79+
remote: "true",
80+
expected: false,
81+
},
82+
{
83+
name: "managed-only parent properties map is skipped",
84+
path: "properties",
85+
remote: map[string]string{"unity.catalog.managed.delta.defaults.delta.enableRowTracking": "true"},
86+
expected: true,
87+
},
88+
{
89+
name: "mixed parent properties map is not skipped",
90+
path: "properties",
91+
remote: map[string]string{"unity.catalog.managed.delta.defaults.delta.enableRowTracking": "true", "custom.remote_only": "true"},
92+
expected: false,
93+
},
94+
}
95+
96+
for _, tt := range tests {
97+
t.Run(tt.name, func(t *testing.T) {
98+
path, err := structpath.ParsePath(tt.path)
99+
require.NoError(t, err)
100+
101+
reason, ok := shouldSkipBackendDefault(cfg, path, &deployplan.ChangeDesc{
102+
Old: nil,
103+
New: nil,
104+
Remote: tt.remote,
105+
})
106+
107+
assert.Equal(t, tt.expected, ok)
108+
if tt.expected {
109+
assert.Equal(t, deployplan.ReasonBackendDefault, reason)
110+
} else {
111+
assert.Empty(t, reason)
112+
}
113+
})
114+
}
115+
}
116+
117+
// Map drift handling synthesizes child paths to match against rules. structdiff
118+
// always emits map keys in bracket notation, so synthetic child paths must too;
119+
// otherwise rules wouldn't match for identifier-like keys.
120+
func TestShouldSkipBackendDefault_MapDriftUsesBracketKeys(t *testing.T) {
121+
field, err := structpath.ParsePattern("properties['simple']")
122+
require.NoError(t, err)
123+
cfg := &dresources.ResourceLifecycleConfig{
124+
BackendDefaults: []dresources.BackendDefaultRule{{Field: field}},
125+
}
126+
127+
path, err := structpath.ParsePath("properties")
128+
require.NoError(t, err)
129+
130+
reason, ok := shouldSkipBackendDefault(cfg, path, &deployplan.ChangeDesc{
131+
Remote: map[string]string{"simple": "v"},
132+
})
133+
assert.True(t, ok)
134+
assert.Equal(t, deployplan.ReasonBackendDefault, reason)
135+
}

bundle/direct/dresources/resources.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,12 @@ resources:
320320
normalize_slash:
321321
- field: storage_root
322322
reason: uc_strips_trailing_slash
323+
backend_defaults:
324+
# UC auto-populates these system-managed keys after create.
325+
# Without this, every subsequent plan produces an Update whose payload is empty,
326+
# and UC rejects it with "UpdateSchema Nothing to update".
327+
- field: properties['unity.catalog.managed.delta.defaults.delta.enableRowTracking']
328+
- field: properties['unity.catalog.managed.iceberg.defaults.delta.feature.catalogManaged']
323329

324330
external_locations:
325331
recreate_on_changes:

libs/testserver/schemas.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ import (
1212

1313
const testMetastoreName = "deco-uc-prod-isolated-aws-us-east-1"
1414

15+
// schemaNameManagedDefaults is the schema name the backend-default drift test uses
16+
// to opt into UC's managed-property simulation. Scoping the injection to this name
17+
// keeps unrelated schema tests free of the property, which terraform would otherwise
18+
// report as drift on redeploy.
19+
const schemaNameManagedDefaults = "schema_managed_defaults"
20+
1521
func (s *FakeWorkspace) SchemasCreate(req Request) Response {
1622
defer s.LockUnlock()()
1723

@@ -42,6 +48,14 @@ func (s *FakeWorkspace) SchemasCreate(req Request) Response {
4248
schema.MetastoreId = TestMetastore.MetastoreId
4349
schema.Owner = s.CurrentUser().UserName
4450
schema.SchemaId = nextUUID()
51+
if schema.Properties == nil && schema.Name == schemaNameManagedDefaults {
52+
// Mirror UC behavior: managed system defaults are populated when the user
53+
// doesn't specify any properties. Required to cover backend-default drift.
54+
schema.Properties = map[string]string{
55+
"unity.catalog.managed.delta.defaults.delta.enableRowTracking": "true",
56+
"unity.catalog.managed.iceberg.defaults.delta.feature.catalogManaged": "true",
57+
}
58+
}
4559
s.Schemas[schema.FullName] = schema
4660

4761
return Response{

0 commit comments

Comments
 (0)