Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged
}
]
}
},
"config.env[0].value": {
"action": "skip",
"reason": "no active deployment",
"old": "original_value",
"new": "changed_value"
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ Plan: 1 to add, 0 to change, 1 to delete, 1 unchanged
}
}
},
"lifecycle.triggers.on_bundle_deploy": {
"action": "recreate",
"reason": "immutable",
"old": "[UUID]",
"new": "[UUID]"
},
"result_state": {
"action": "skip",
"reason": "remote_already_set",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,6 @@
"timezone_id": "America/New_York"
}
},
"schedule.timezone_id": {
"action": "update",
"old": "UTC",
"new": "America/New_York"
},
"tasks[task_key='main'].email_notifications": {
"action": "skip",
"reason": "empty",
Expand Down
7 changes: 3 additions & 4 deletions acceptance/bundle/resources/jobs/whole-block-overlap/script
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ r["schedule"] = None
EOF

# The local diff is at the leaf (schedule.timezone_id) while the remote diff is at
# the block (schedule = nil vs the configured block). structdiff records the remote
# side as one block-level change instead of descending, so the plan's "changes"
# carries BOTH the whole "schedule" block and the "schedule.timezone_id" leaf
# instead of merging them into leaves.
# the block (schedule = nil vs the configured block). The block-level change wins:
# the plan's "changes" keeps one level per subtree (the whole "schedule" block), not
# an overlapping block + "schedule.timezone_id" leaf pair.
trace $CLI bundle plan
$CLI bundle plan -o json | nostamp > out.plan.direct.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Direct-only: the overlapping parent+child entries are a direct-engine changes-map
# artifact (terraform emits no per-field changes).
# Direct-only: the per-field "changes" map is a direct-engine artifact (terraform
# emits no per-field changes). A local leaf change plus a whole block dropped on the
# remote merge into a single block-level entry (one level per subtree), not an
# overlapping block + child pair.
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]
RecordRequests = false
Ignore = [".databricks"]

Badness = "The plan's 'changes' map carries BOTH the whole 'schedule' block and the 'schedule.timezone_id' leaf. The local diff (saved state vs config) is at the leaf while the remote diff (remote vs config) is at the block level (schedule dropped out-of-band), and structdiff records the remote side as one block-level change instead of descending, so the two never merge into leaves."
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged
{
"telemetry_config": {
"action": "skip",
"reason": "backend_default",
"reason": "input_only",
"remote": {
"telemetry_profile_id": "[UUID]"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"serial": 1,
"plan": {
"resources.pipelines.my": {
"action": "update",
"action": "recreate",
"new_state": {
"value": {
"channel": "CURRENT",
Expand Down Expand Up @@ -57,7 +57,8 @@
},
"changes": {
"ingestion_definition": {
"action": "update",
"action": "recreate",
"reason": "immutable",
"new": {
"connection_name": "my_connection",
"objects": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ trace $CLI bundle deploy
# markers. connection_name is immutable, so adding it must recreate the pipeline.
trace update_file.py databricks.yml "#TO_ADD: " ""

# The direct engine records the whole ingestion_definition block as one 'update'
# entry (missing the recreate rule on the connection_name leaf); terraform records
# 'recreate'. See the per-engine "changes" / "action" in the plan JSON.
# Both engines recreate. structdiff records the added block as one change at
# "ingestion_definition"; bidirectional rule matching relates that to the recreate
# rule on the connection_name leaf. See the per-engine "changes"/"action" in the JSON.
$CLI bundle plan -o json | nostamp > out.plan.$DATABRICKS_BUNDLE_ENGINE.json

rm -f out.requests.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Runs on both engines to show the difference: the direct engine plans this as
# 'update' (see Badness), while terraform correctly recreates. The plan verdict
# is the only divergent output, so it is routed to per-engine out.plan.*.txt.
Badness = "On the direct engine, adding the whole ingestion_definition block is planned as 'update' instead of 'recreate': structdiff records one change at path 'ingestion_definition' instead of descending to 'ingestion_definition.connection_name', so the immutable recreate rule never matches. Terraform recreates as expected."
# Runs on both engines: both recreate (connection_name is immutable). The direct
# plan's per-engine "changes" map (out.plan.direct.json) shows the recreate keyed on
# the ingestion_definition block via bidirectional rule matching; terraform's plan
# JSON just carries the action.
Ignore = ["foo.py", ".databricks"]
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"serial": 1,
"plan": {
"resources.pipelines.my": {
"action": "update",
"action": "recreate",
"new_state": {
"value": {
"channel": "CURRENT",
Expand Down Expand Up @@ -57,7 +57,8 @@
},
"changes": {
"ingestion_definition": {
"action": "update",
"action": "recreate",
"reason": "immutable",
"old": {
"connection_name": "my_connection",
"objects": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ trace $CLI bundle deploy
# lines. connection_name is immutable, so removing it must recreate the pipeline.
grep -v '# TO_DELETE' databricks.yml > databricks.yml.tmp && mv databricks.yml.tmp databricks.yml

# The direct engine records the whole ingestion_definition block as one 'update'
# entry (missing the recreate rule on the connection_name leaf); terraform records
# 'recreate'. See the per-engine "changes" / "action" in the plan JSON.
# Both engines recreate. structdiff records the removed block as one change at
# "ingestion_definition"; bidirectional rule matching relates that to the recreate
# rule on the connection_name leaf. See the per-engine "changes"/"action" in the JSON.
$CLI bundle plan -o json | nostamp > out.plan.$DATABRICKS_BUNDLE_ENGINE.json

rm -f out.requests.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Runs on both engines to show the difference: the direct engine plans this as
# 'update' (see Badness), while terraform correctly recreates. The plan verdict
# is the only divergent output, so it is routed to per-engine out.plan.*.txt.
Badness = "On the direct engine, removing the whole ingestion_definition block is planned as 'update' instead of 'recreate': structdiff records one change at path 'ingestion_definition' instead of descending to 'ingestion_definition.connection_name', so the immutable recreate rule never matches. Terraform recreates as expected."
# Runs on both engines: both recreate (connection_name is immutable). The direct
# plan's per-engine "changes" map (out.plan.direct.json) shows the recreate keyed on
# the ingestion_definition block via bidirectional rule matching; terraform's plan
# JSON just carries the action.
Ignore = ["foo.py", ".databricks"]
44 changes: 43 additions & 1 deletion bundle/direct/bundle_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,30 @@ func prepareChanges(ctx context.Context, adapter *dresources.Adapter, localDiff,
}
}

// Keep one level per subtree. A whole block added or removed shows up as a single
// block-level change, while the other diff (local vs remote) may have descended to
// inner fields of the same subtree. The block-level change already describes them,
// so drop any entry that has an ancestor entry in the map.
for pathStr := range m {
node, err := structpath.ParsePath(pathStr)
if err != nil {
continue
}
for other := range m {
if other == pathStr {
continue
}
ancestor, err := structpath.ParsePath(other)
if err != nil {
continue
}
if node.HasPrefix(ancestor) {
delete(m, pathStr)
break
}
}
}

return m, nil
}

Expand Down Expand Up @@ -550,13 +574,31 @@ func isFieldMissingInRemote(adapter *dresources.Adapter, path *structpath.PathNo

func findMatchingRule(path *structpath.PathNode, rules []dresources.FieldRule) (string, bool) {
for _, r := range rules {
if path.HasPatternPrefix(r.Field) {
if matchesFieldRule(path, r.Field) {
return r.Reason, true
}
}
return "", false
}

// matchesFieldRule reports whether a rule targeting the pattern matches a change at
// path, in either direction:
// - path at or below the rule: the field the rule names changed (the usual case).
// - path above the rule: a whole block was added or removed, recorded as one
// block-level change, so the field the rule names is part of it.
//
// The second direction is what lets a rule on ingestion_definition.connection_name
// match a whole-block change recorded at ingestion_definition.
func matchesFieldRule(path *structpath.PathNode, pattern *structpath.PatternNode) bool {
if path.HasPatternPrefix(pattern) {
return true
}
if path.Len() < pattern.Len() {
return path.HasPatternPrefix(pattern.Prefix(path.Len()))
}
return false
}

func shouldSkip(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNode, ch *deployplan.ChangeDesc) (string, bool) {
if cfg == nil {
return "", false
Expand Down
9 changes: 4 additions & 5 deletions bundle/direct/bundle_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,10 +451,9 @@ func TestPrepareChangesWholeBlockOverlap(t *testing.T) {
}
slices.Sort(keys)

// Unexpected: a coarse "field.a" entry overlaps the fine "field.a.c" entry.
// Probably should be: []string{"field.a.b", "field.a.c"}.
assert.Equal(t, []string{"field.a", "field.a.c"}, keys)

// The coarse parent entry carries the whole sub-block rather than a leaf value.
// One level per subtree: the local diff produced the field.a.c leaf and the remote
// diff the whole field.a block; the block-level entry wins and the inner leaf is
// dropped, so the merged change set is just the block.
assert.Equal(t, []string{"field.a"}, keys)
assert.Equal(t, threeWayInner{B: "old", C: "newc"}, changes["field.a"].New)
}
Loading