From 36cc95b99fc31372c47bd302d587ef33a257a645 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Sun, 2 Aug 2026 19:56:30 +0000 Subject: [PATCH] bundle: write config sync key renames as a key rewrite Co-authored-by: Isaac --- .../config-remote-sync/job_fields/output.txt | 10 +- .../job_multiple_tasks/output.txt | 13 +- .../multiple_files/output.txt | 32 +-- .../split/isolation/output.txt | 24 +- .../split/keyed_rename/databricks.yml.tmpl | 43 ++++ .../split/keyed_rename/out.test.toml | 4 + .../split/keyed_rename/output.txt | 115 ++++++++++ .../split/keyed_rename/script | 86 +++++++ .../task_rename_revert/output.txt | 30 +-- bundle/configsync/blockindex.go | 5 +- bundle/configsync/rename.go | 212 ++++++++++++++++++ bundle/configsync/resolve.go | 52 ++++- 12 files changed, 542 insertions(+), 84 deletions(-) create mode 100644 acceptance/bundle/config-remote-sync/split/keyed_rename/databricks.yml.tmpl create mode 100644 acceptance/bundle/config-remote-sync/split/keyed_rename/out.test.toml create mode 100644 acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt create mode 100644 acceptance/bundle/config-remote-sync/split/keyed_rename/script create mode 100644 bundle/configsync/rename.go diff --git a/acceptance/bundle/config-remote-sync/job_fields/output.txt b/acceptance/bundle/config-remote-sync/job_fields/output.txt index 7e0b7fe07d4..9e4f05db576 100644 --- a/acceptance/bundle/config-remote-sync/job_fields/output.txt +++ b/acceptance/bundle/config-remote-sync/job_fields/output.txt @@ -58,18 +58,14 @@ Resource: resources.jobs.my_job + - samples.nyctaxi.trips environments: - environment_key: default -@@ -25,14 +31,14 @@ +@@ -25,5 +31,5 @@ - ./*.whl job_clusters: - - job_cluster_key: test_cluster + - job_cluster_key: test_cluster_renamed new_cluster: -- spark_version: [[DEFAULT_SPARK_VERSION]] - node_type_id: [NODE_TYPE_ID] - num_workers: 1 -+ spark_version: [[DEFAULT_SPARK_VERSION]] - tasks: - - task_key: main + spark_version: [[DEFAULT_SPARK_VERSION]] +@@ -34,5 +40,5 @@ notebook_task: notebook_path: /Users/{{workspace_user_name}}/notebook - job_cluster_key: test_cluster diff --git a/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt b/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt index 6c45a66bed1..39ea29bf4a5 100644 --- a/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt +++ b/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt @@ -97,21 +97,14 @@ Resource: resources.jobs.rename_task_job >>> diff.py databricks.yml.backup2 databricks.yml --- databricks.yml.backup2 +++ databricks.yml -@@ -52,14 +52,14 @@ +@@ -52,5 +52,5 @@ rename_task_job: tasks: - - task_key: b_task -+ - new_cluster: -+ node_type_id: [NODE_TYPE_ID] -+ num_workers: 1 -+ spark_version: 13.3.x-snapshot-scala2.12 ++ - task_key: b_task_renamed notebook_task: notebook_path: /Users/{{workspace_user_name}}/b_task -- new_cluster: -- spark_version: 13.3.x-snapshot-scala2.12 -- node_type_id: [NODE_TYPE_ID] -- num_workers: 1 -+ task_key: b_task_renamed +@@ -61,5 +61,5 @@ - task_key: d_task depends_on: - - task_key: b_task diff --git a/acceptance/bundle/config-remote-sync/multiple_files/output.txt b/acceptance/bundle/config-remote-sync/multiple_files/output.txt index aa943ca185a..2799e540f41 100644 --- a/acceptance/bundle/config-remote-sync/multiple_files/output.txt +++ b/acceptance/bundle/config-remote-sync/multiple_files/output.txt @@ -32,27 +32,13 @@ Resource: resources.jobs.job_two >>> diff.py resources/job1.yml.backup resources/job1.yml --- resources/job1.yml.backup +++ resources/job1.yml -@@ -4,13 +4,13 @@ +@@ -4,5 +4,5 @@ max_concurrent_runs: 1 tasks: - - task_key: c_task -+ - depends_on: -+ - task_key: b_task -+ new_cluster: -+ node_type_id: [NODE_TYPE_ID] -+ num_workers: 1 -+ spark_version: 13.3.x-snapshot-scala2.12 ++ - task_key: c_task_renamed notebook_task: notebook_path: /Users/{{workspace_user_name}}/c_task -- new_cluster: -- spark_version: 13.3.x-snapshot-scala2.12 -- node_type_id: [NODE_TYPE_ID] -- num_workers: 1 -- depends_on: -- - task_key: b_task -+ task_key: c_task_renamed - - task_key: a_task - notebook_task: @@ -21,3 +21,10 @@ num_workers: 1 depends_on: @@ -71,26 +57,16 @@ Resource: resources.jobs.job_two >>> diff.py resources/job2.yml.backup resources/job2.yml --- resources/job2.yml.backup +++ resources/job2.yml -@@ -2,13 +2,13 @@ +@@ -2,7 +2,7 @@ jobs: job_two: - max_concurrent_runs: 2 + max_concurrent_runs: 10 tasks: - - task_key: run_pipeline -+ - new_cluster: -+ node_type_id: [NODE_TYPE_ID] -+ num_workers: 1 -+ spark_version: 13.3.x-snapshot-scala2.12 ++ - task_key: run_pipeline_renamed notebook_task: notebook_path: /Users/{{workspace_user_name}}/1 -- new_cluster: -- spark_version: 13.3.x-snapshot-scala2.12 -- node_type_id: [NODE_TYPE_ID] -- num_workers: 1 -+ task_key: run_pipeline_renamed - - task_key: etl_pipeline - notebook_task: @@ -18,5 +18,9 @@ node_type_id: [NODE_TYPE_ID] num_workers: 2 diff --git a/acceptance/bundle/config-remote-sync/split/isolation/output.txt b/acceptance/bundle/config-remote-sync/split/isolation/output.txt index 400aa2c152f..20ecba57699 100644 --- a/acceptance/bundle/config-remote-sync/split/isolation/output.txt +++ b/acceptance/bundle/config-remote-sync/split/isolation/output.txt @@ -21,36 +21,32 @@ Resource: resources.jobs.job_b >>> diff.py databricks.yml.backup databricks.yml --- databricks.yml.backup +++ databricks.yml -@@ -12,11 +12,11 @@ +@@ -12,5 +12,5 @@ job_a: tasks: - - task_key: shared -- max_retries: 1 -+ - max_retries: 1 ++ - task_key: shared_renamed + max_retries: 1 notebook_task: -- notebook_path: /Users/{{workspace_user_name}}/shared -- -+ notebook_path: '/Users/{{workspace_user_name}}/shared' -+ task_key: shared_renamed -+ timeout_seconds: 45 +@@ -18,5 +18,5 @@ + job_b: - max_concurrent_runs: 1 + max_concurrent_runs: 6 tasks: - task_key: simple -@@ -30,5 +30,3 @@ - jobs: +@@ -31,4 +31,4 @@ job_a: -- tasks: + tasks: - - task_key: shared -- timeout_seconds: 45 -+ tasks: [] ++ - task_key: shared_renamed + timeout_seconds: 45 >>> grep -c max_concurrent_runs: 6 databricks.yml 1 >>> grep -c task_key: shared_renamed databricks.yml -1 +2 >>> [CLI] bundle destroy --auto-approve -t dev The following resources will be deleted: diff --git a/acceptance/bundle/config-remote-sync/split/keyed_rename/databricks.yml.tmpl b/acceptance/bundle/config-remote-sync/split/keyed_rename/databricks.yml.tmpl new file mode 100644 index 00000000000..9648ab01b5c --- /dev/null +++ b/acceptance/bundle/config-remote-sync/split/keyed_rename/databricks.yml.tmpl @@ -0,0 +1,43 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +# "shared" is defined in both blocks: max_retries at the top level and +# timeout_seconds in the target override. Renaming it must rewrite the key in both +# places and leave each field in the scope it was written in. +# +# "solo" lives only in the target block, and the new keys are chosen so their sort +# order is the reverse of the block order: "a_shared" sorts before "z_solo", while +# the top-level block comes before the target block. An implementation that pairs +# renames by order rather than by identity swaps their contents. +resources: + jobs: + rename_job: + tasks: + - task_key: shared + max_retries: 1 + notebook_task: + notebook_path: /Users/{{workspace_user_name}}/shared + + # A single block, to check that a removal earlier in the list shifts the + # position a later rename has to be written at. + shift_job: + tasks: + - task_key: aaa + max_retries: 1 + - task_key: mmm + max_retries: 2 + - task_key: zzz + max_retries: 3 + +targets: + dev: + mode: development + resources: + jobs: + rename_job: + tasks: + - task_key: shared + timeout_seconds: 45 + - task_key: solo + notebook_task: + notebook_path: /Users/{{workspace_user_name}}/solo diff --git a/acceptance/bundle/config-remote-sync/split/keyed_rename/out.test.toml b/acceptance/bundle/config-remote-sync/split/keyed_rename/out.test.toml new file mode 100644 index 00000000000..4b5914daa2c --- /dev/null +++ b/acceptance/bundle/config-remote-sync/split/keyed_rename/out.test.toml @@ -0,0 +1,4 @@ +Local = true +Cloud = false +GOOS.windows = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct", "terraform"] diff --git a/acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt b/acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt new file mode 100644 index 00000000000..926834b83ef --- /dev/null +++ b/acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt @@ -0,0 +1,115 @@ +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/dev/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +=== Rename a task defined in both blocks and a target-only task +=== Sync +Detected changes in 1 resource(s): + +Resource: resources.jobs.rename_job + tasks[task_key='a_shared']: add + tasks[task_key='shared']: remove + tasks[task_key='solo']: remove + tasks[task_key='z_solo']: add + + + +=== Both blocks get the new key; each field keeps its own scope + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -14,5 +14,5 @@ + rename_job: + tasks: +- - task_key: shared ++ - task_key: a_shared + max_retries: 1 + notebook_task: +@@ -37,7 +37,7 @@ + rename_job: + tasks: +- - task_key: shared ++ - task_key: a_shared + timeout_seconds: 45 +- - task_key: solo ++ - task_key: z_solo + notebook_task: + notebook_path: /Users/{{workspace_user_name}}/solo + +>>> grep -c task_key: a_shared databricks.yml +2 + +>>> grep -c task_key: z_solo databricks.yml +1 + +=== Remove a task and rename a later one in the same run, single block +=== Sync +Detected changes in 1 resource(s): + +Resource: resources.jobs.shift_job + tasks[task_key='aaa']: remove + tasks[task_key='mmm']: remove + tasks[task_key='mmm2']: add + + + +=== aaa is gone, mmm became mmm2, zzz keeps its name and its max_retries + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -23,7 +23,5 @@ + shift_job: + tasks: +- - task_key: aaa +- max_retries: 1 +- - task_key: mmm ++ - task_key: mmm2 + max_retries: 2 + - task_key: zzz + +>>> grep -c task_key: zzz databricks.yml +1 + +=== Rename the two-block task AND edit one of its fields in the same run +=== Sync +Detected changes in 1 resource(s): + +Resource: resources.jobs.rename_job + tasks[task_key='a_shared']: remove + tasks[task_key='b_shared']: add + tasks[task_key='z_solo']: replace + + + +=== Left unapplied: the split is intact and timeout_seconds stays target-scoped + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -37,5 +37,5 @@ + - task_key: a_shared + timeout_seconds: 45 +- - task_key: z_solo +- notebook_task: ++ - notebook_task: + notebook_path: /Users/{{workspace_user_name}}/solo ++ task_key: z_solo + +>>> grep -c task_key: a_shared databricks.yml +2 + +>>> grep -c timeout_seconds: 45 databricks.yml +1 + +>>> [CLI] bundle destroy --auto-approve -t dev +The following resources will be deleted: + delete resources.jobs.rename_job + delete resources.jobs.shift_job + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/dev + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/config-remote-sync/split/keyed_rename/script b/acceptance/bundle/config-remote-sync/split/keyed_rename/script new file mode 100644 index 00000000000..88b8106ed55 --- /dev/null +++ b/acceptance/bundle/config-remote-sync/split/keyed_rename/script @@ -0,0 +1,86 @@ +#!/bin/bash + +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve -t dev +} +trap cleanup EXIT + +$CLI bundle deploy -t dev +job_id="$(read_id.py rename_job)" +shift_job_id="$(read_id.py shift_job)" + + +# Rename the two-block task and the target-only task in one run. Each key rewrite +# has to land on its own element: "a_shared" in both blocks, "z_solo" in the target +# block only. Their sort order is the reverse of the block order, so pairing by +# order instead of by identity swaps the two. +title "Rename a task defined in both blocks and a target-only task" +edit_resource.py jobs $job_id <>> diff.py databricks.yml.backup databricks.yml --- databricks.yml.backup +++ databricks.yml -@@ -6,11 +6,11 @@ +@@ -6,5 +6,5 @@ sample_job: tasks: - - task_key: new_task -- notebook_task: -- notebook_path: /Users/{{workspace_user_name}}/new_task -- new_cluster: -- spark_version: 13.3.x-snapshot-scala2.12 -+ - new_cluster: - node_type_id: [NODE_TYPE_ID] - num_workers: 1 -+ spark_version: 13.3.x-snapshot-scala2.12 -+ notebook_task: -+ notebook_path: '/Users/{{workspace_user_name}}/new_task' -+ task_key: new_task_2 - - targets: ++ - task_key: new_task_2 + notebook_task: + notebook_path: /Users/{{workspace_user_name}}/new_task === Rename task back to new_task remotely @@ -49,13 +39,13 @@ Resource: resources.jobs.sample_job >>> diff.py databricks.yml.backup databricks.yml --- databricks.yml.backup +++ databricks.yml -@@ -12,5 +12,5 @@ +@@ -6,5 +6,5 @@ + sample_job: + tasks: +- - task_key: new_task_2 ++ - task_key: new_task notebook_task: - notebook_path: '/Users/{{workspace_user_name}}/new_task' -- task_key: new_task_2 -+ task_key: new_task - - targets: + notebook_path: /Users/{{workspace_user_name}}/new_task >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: diff --git a/bundle/configsync/blockindex.go b/bundle/configsync/blockindex.go index 911b31b73fb..aefd950081d 100644 --- a/bundle/configsync/blockindex.go +++ b/bundle/configsync/blockindex.go @@ -267,8 +267,9 @@ type routeDestination struct { // routeElement maps a change that addresses a whole sequence element onto every // block that defines it. An element assembled from two blocks has a part in each, -// so removing it means deleting both parts. Expressing the change per block keeps -// the split intact instead of collapsing the element into one scope. +// so removing it means deleting both parts, and renaming it means rewriting the +// key in both. Expressing the change per block keeps the split intact instead of +// collapsing the element into one scope. func (r *blockResolver) routeElement(change resolvedChange) ([]routeDestination, error) { if len(change.steps) == 0 { return nil, fmt.Errorf("%w: change does not address a sequence element", errAmbiguousBlock) diff --git a/bundle/configsync/rename.go b/bundle/configsync/rename.go new file mode 100644 index 00000000000..275440dab74 --- /dev/null +++ b/bundle/configsync/rename.go @@ -0,0 +1,212 @@ +package configsync + +import ( + "errors" + "fmt" + "maps" + "reflect" + "slices" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/structs/structpath" +) + +// renamedElement is a keyed sequence element whose key changed remotely. +type renamedElement struct { + // keyField is the field holding the key, e.g. "task_key". + keyField string + oldKey string + newKey string + // addPath is the change path of the add half, kept so it can be skipped. + addPath string +} + +type renameSet struct { + // byRemovePath maps the remove half's change path to the pair. + byRemovePath map[string]renamedElement + // addPaths are the add halves, which must not be routed on their own. + addPaths map[string]struct{} + // unpairedPaths are the halves of a suspected key change on an element that + // is defined in several blocks, where the two halves could not be matched. + // Applying them separately would delete the element from every block and + // recreate it in one, collapsing the split and moving fields into a scope the + // user did not choose, so neither half is applied. + unpairedPaths map[string]struct{} +} + +// pairRenames matches removes of keyed elements against adds in the same sequence +// that carry the same content apart from the key. +// +// A remote key change is reported as an unrelated remove plus add, so without +// pairing the element is deleted and recreated: the recreated copy has to be +// placed somewhere, and for an element defined in several blocks there is no +// single right place. Recognising the pair turns it into a key rewrite, which +// every defining block can apply to its own part. +func pairRenames(b *bundle.Bundle, blocks *blockResolver, resourceKey string, changes ResourceChanges) renameSet { + set := renameSet{ + byRemovePath: map[string]renamedElement{}, + addPaths: map[string]struct{}{}, + unpairedPaths: map[string]struct{}{}, + } + if blocks == nil { + return set + } + + removes, adds := keyedElementChanges(changes) + for _, remove := range removes { + paired := false + for _, add := range adds { + if _, taken := set.addPaths[add.path]; taken { + continue + } + if remove.parent != add.parent || remove.keyField != add.keyField { + continue + } + if !sameElementApartFromKey(b, resourceKey, remove, add) { + continue + } + set.byRemovePath[remove.path] = renamedElement{ + keyField: remove.keyField, + oldKey: remove.key, + newKey: add.key, + addPath: add.path, + } + set.addPaths[add.path] = struct{}{} + paired = true + break + } + if paired { + continue + } + + // The removal was not matched to an addition. A plain removal is fine: it + // deletes every part of the element, which is what the user asked for. + // But if an unmatched addition to the same sequence is present, the two are + // most likely one key change whose element also had a field edited. Applying + // them separately would delete a split element from every block and recreate + // it in one, collapsing the split and moving fields into a scope the user did + // not choose, so hold both halves back. + var candidates []string + for _, add := range adds { + if remove.parent != add.parent || remove.keyField != add.keyField { + continue + } + if _, taken := set.addPaths[add.path]; !taken { + candidates = append(candidates, add.path) + } + } + if len(candidates) == 0 { + continue + } + if !multiBlockElement(b, blocks, resourceKey, remove.path) { + continue + } + set.unpairedPaths[remove.path] = struct{}{} + for _, path := range candidates { + set.unpairedPaths[path] = struct{}{} + } + } + return set +} + +// multiBlockElement reports whether the element at path is assembled from more +// than one physical block. +func multiBlockElement(b *bundle.Bundle, blocks *blockResolver, resourceKey, path string) bool { + resolved, err := resolveSelectors(resourceKey+"."+path, b, OperationRemove) + if err != nil || len(resolved.steps) == 0 { + return false + } + last := resolved.steps[len(resolved.steps)-1] + return len(blocks.blocksOf(last.element)) > 1 +} + +// keyedElement is one side of a candidate rename. +type keyedElement struct { + path string + parent string + keyField string + key string + value any +} + +// keyedElementChanges splits the changes that address a whole keyed element into +// removes and adds, in a deterministic order. +func keyedElementChanges(changes ResourceChanges) (removes, adds []keyedElement) { + for _, path := range slices.Sorted(maps.Keys(changes)) { + change := changes[path] + if change.Operation != OperationRemove && change.Operation != OperationAdd { + continue + } + node, err := structpath.ParsePath(path) + if err != nil { + continue + } + keyField, key, ok := node.KeyValue() + if !ok { + continue + } + element := keyedElement{ + path: path, + parent: node.Parent().String(), + keyField: keyField, + key: key, + value: change.Value, + } + if change.Operation == OperationRemove { + removes = append(removes, element) + } else { + adds = append(adds, element) + } + } + return removes, adds +} + +// sameElementApartFromKey reports whether the added element is the removed one +// with a different key. The remove half carries no value, so the old element is +// read from the merged configuration. +func sameElementApartFromKey(b *bundle.Bundle, resourceKey string, remove, add keyedElement) bool { + resolved, err := resolveSelectors(resourceKey+"."+remove.path, b, OperationRemove) + if err != nil || !resolved.leaf.IsValid() { + return false + } + oldValue, ok := resolved.leaf.AsAny().(map[string]any) + if !ok { + return false + } + newValue, ok := add.value.(map[string]any) + if !ok { + return false + } + return reflect.DeepEqual(withoutKey(oldValue, remove.keyField), withoutKey(newValue, add.keyField)) +} + +func withoutKey(value map[string]any, keyField string) map[string]any { + out := make(map[string]any, len(value)) + for field, fieldValue := range value { + if field != keyField { + out[field] = fieldValue + } + } + return out +} + +// routeRenameElement locates the renamed element in every block that defines it. +// The caller turns each destination into a rewrite of the key field, so the +// element's other fields stay where they are and a split element keeps its parts +// in their original scopes. +func routeRenameElement(b *bundle.Bundle, blocks *blockResolver, resourceKey, removePath string) ([]routeDestination, error) { + fullPath := resourceKey + "." + removePath + resolved, err := resolveSelectors(fullPath, b, OperationRemove) + if err != nil { + return nil, fmt.Errorf("failed to resolve selectors in path %s: %w", fullPath, err) + } + + destinations, err := blocks.routeElement(resolved) + if err != nil { + if errors.Is(err, errAmbiguousBlock) { + return nil, nil + } + return nil, fmt.Errorf("failed to route rename %s: %w", fullPath, err) + } + return destinations, nil +} diff --git a/bundle/configsync/resolve.go b/bundle/configsync/resolve.go index fa3bc6c8bc8..30570a00824 100644 --- a/bundle/configsync/resolve.go +++ b/bundle/configsync/resolve.go @@ -227,6 +227,12 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes return cmp.Compare(a, b) }) + // A key change on a keyed element arrives as a remove of the old key plus an + // add of the new one, with nothing linking them. Pairing them back up lets a + // rename be written as a key rewrite in every block that defines the element, + // which keeps a split element split instead of collapsing it into one scope. + renames := pairRenames(b, blocks, resourceKey, resourceChanges) + // Create indices map for this resource, path -> indices, that we could use to replace with added elements indicesToReplaceMap := make(map[string][]int) @@ -239,6 +245,17 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes configChange := resourceChanges[fieldPath] fullPath := resourceKey + "." + fieldPath + // The add half of a rename carries no source location of its own; the + // pair is written from the remove half, which does. + if _, ok := renames.addPaths[fieldPath]; ok { + continue + } + if _, ok := renames.unpairedPaths[fieldPath]; ok { + log.Debugf(ctx, "config-remote-sync: skipping %s: a split element cannot be removed and recreated in one run", fullPath) + continue + } + rename, isRename := renames.byRemovePath[fieldPath] + resolved, err := resolveSelectors(fullPath, b, configChange.Operation) if err != nil { return nil, fmt.Errorf("failed to resolve selectors in path %s: %w", fullPath, err) @@ -249,14 +266,22 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes // the physical block that defines it, because the merged order and the // per-block order differ once a sequence is split across blocks. An // element assembled from several blocks has a part in each, so removing - // it yields one destination per block. + // or renaming it yields one destination per block. destinations := []routeDestination{{path: resolvedPath}} routed := false if blocks != nil && len(resolved.steps) > 0 { var routeErr error - if configChange.Operation == OperationRemove { + switch { + case isRename: + destinations, routeErr = routeRenameElement(b, blocks, resourceKey, fieldPath) + if routeErr == nil && len(destinations) == 0 { + // The element could not be attributed to a block; leave the + // whole pair for a later run rather than half-applying it. + continue + } + case configChange.Operation == OperationRemove: destinations, routeErr = blocks.routeElement(resolved) - } else { + default: var block sourceBlock var path *structpath.PatternNode block, path, routeErr = blocks.route(resolved) @@ -293,6 +318,27 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes scope = block.prefix + "\x00" + block.file + "\x00" } + // A rename rewrites only the key field of the element, so it is a + // replace at the element's position rather than a change to the + // element itself. The index is still adjusted below, because + // removals earlier in the same block shift it. + if isRename { + resolvedPath = adjustArrayIndex(resolvedPath, scope, indexOperations) + destChange.Operation = OperationReplace + destChange.Value = rename.newKey + resolvedPath = structpath.NewPatternStringKey(resolvedPath, rename.keyField) + candidate := resolvedPath.String() + if block.prefix != "" { + candidate = block.prefix + "." + candidate + } + result = append(result, FieldChange{ + FilePath: block.file, + Change: destChange, + FieldCandidates: []string{candidate}, + }) + continue + } + // If the element is removed, we can use the index to replace it with added element // That may improve the diff in cases when the task is recreated because of renaming if destChange.Operation == OperationRemove {