diff --git a/acceptance/bundle/config-remote-sync/resource_keys/databricks.yml.tmpl b/acceptance/bundle/config-remote-sync/resource_keys/databricks.yml.tmpl new file mode 100644 index 00000000000..51886911999 --- /dev/null +++ b/acceptance/bundle/config-remote-sync/resource_keys/databricks.yml.tmpl @@ -0,0 +1,27 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +variables: + notebook_root: + default: /Users/{{workspace_user_name}} + +resources: + jobs: + "job with spaces": + name: spaced-job + tasks: + - task_key: main + timeout_seconds: 60 + notebook_task: + notebook_path: ${var.notebook_root}/notebook + "job's\"quotes": + name: quoted-job + tasks: + - task_key: main + timeout_seconds: 60 + notebook_task: + notebook_path: ${var.notebook_root}/notebook + +targets: + default: + mode: development diff --git a/acceptance/bundle/config-remote-sync/resource_keys/out.test.toml b/acceptance/bundle/config-remote-sync/resource_keys/out.test.toml new file mode 100644 index 00000000000..8c2cb31de53 --- /dev/null +++ b/acceptance/bundle/config-remote-sync/resource_keys/out.test.toml @@ -0,0 +1,3 @@ +Cloud = false +GOOS.windows = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/config-remote-sync/resource_keys/output.txt b/acceptance/bundle/config-remote-sync/resource_keys/output.txt new file mode 100644 index 00000000000..e7de6210739 --- /dev/null +++ b/acceptance/bundle/config-remote-sync/resource_keys/output.txt @@ -0,0 +1,110 @@ + +>>> [CLI] bundle deploy -qq + +=== Edit job with spaces remotely +=== Sync only job with spaces +>>> [CLI] bundle config-remote-sync --select-ids jobs:[JOB WITH SPACES_ID] --save +Detected changes in 1 resource(s): + +Resource: resources.jobs.job with spaces + description: add + tasks[task_key='main'].notebook_task.notebook_path: replace + tasks[task_key='main'].timeout_seconds: replace + + + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -12,7 +12,8 @@ + tasks: + - task_key: main +- timeout_seconds: 60 ++ timeout_seconds: 900 + notebook_task: +- notebook_path: ${var.notebook_root}/notebook ++ notebook_path: ${var.notebook_root}/notebook_updated ++ description: Remote description + "job's\"quotes": + name: quoted-job + +=== Syncing job with spaces again is idempotent +>>> [CLI] bundle config-remote-sync --select-ids jobs:[JOB WITH SPACES_ID] --save +No changes detected. + + +=== Add a sibling task in job with spaces +>>> [CLI] bundle config-remote-sync --select-ids jobs:[JOB WITH SPACES_ID] --save +Detected changes in 1 resource(s): + +Resource: resources.jobs.job with spaces + tasks[task_key='added']: add + + + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -15,4 +15,7 @@ + notebook_task: + notebook_path: ${var.notebook_root}/notebook_updated ++ - notebook_task: ++ notebook_path: ${var.notebook_root}/notebook_updated_sibling ++ task_key: added + description: Remote description + "job's\"quotes": + +=== Edit job's"quotes remotely +=== Sync only job's"quotes +>>> [CLI] bundle config-remote-sync --select-ids jobs:[JOB'S\"QUOTES_ID] --save +Detected changes in 1 resource(s): + +Resource: resources.jobs.job's"quotes + description: add + tasks[task_key='main'].notebook_task.notebook_path: replace + tasks[task_key='main'].timeout_seconds: replace + + + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -23,7 +23,8 @@ + tasks: + - task_key: main +- timeout_seconds: 60 ++ timeout_seconds: 900 + notebook_task: +- notebook_path: ${var.notebook_root}/notebook ++ notebook_path: ${var.notebook_root}/notebook_updated ++ description: Remote description + + targets: + +=== Syncing job's"quotes again is idempotent +>>> [CLI] bundle config-remote-sync --select-ids jobs:[JOB'S\"QUOTES_ID] --save +No changes detected. + + +=== Add a sibling task in job's"quotes +>>> [CLI] bundle config-remote-sync --select-ids jobs:[JOB'S\"QUOTES_ID] --save +Detected changes in 1 resource(s): + +Resource: resources.jobs.job's"quotes + tasks[task_key='added']: add + + + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -26,4 +26,7 @@ + notebook_task: + notebook_path: ${var.notebook_root}/notebook_updated ++ - notebook_task: ++ notebook_path: ${var.notebook_root}/notebook_updated_sibling ++ task_key: added + description: Remote description + + +>>> [CLI] bundle destroy --auto-approve -qq diff --git a/acceptance/bundle/config-remote-sync/resource_keys/script b/acceptance/bundle/config-remote-sync/resource_keys/script new file mode 100644 index 00000000000..4837c6a0c10 --- /dev/null +++ b/acceptance/bundle/config-remote-sync/resource_keys/script @@ -0,0 +1,45 @@ +#!/bin/bash + +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve -qq +} +trap cleanup EXIT + +trace $CLI bundle deploy -qq + +for key in 'job with spaces' "job's\"quotes"; do + job_id="$(read_id.py "$key")" + + title "Edit $key remotely" + edit_resource.py jobs "$job_id" <<'PYEDIT' +r["tasks"][0]["timeout_seconds"] = 900 +r["tasks"][0]["notebook_task"]["notebook_path"] += "_updated" +r["description"] = "Remote description" +PYEDIT + + title "Sync only $key" + cp databricks.yml databricks.yml.backup + trace $CLI bundle config-remote-sync --select-ids "jobs:$job_id" --save + trace diff.py databricks.yml.backup databricks.yml + rm databricks.yml.backup + + title "Syncing $key again is idempotent" + trace $CLI bundle config-remote-sync --select-ids "jobs:$job_id" --save | contains.py "No changes detected." + + title "Add a sibling task in $key" + edit_resource.py jobs "$job_id" <<'PYEDIT' +r["tasks"].append({ + "task_key": "added", + "notebook_task": { + "notebook_path": r["tasks"][0]["notebook_task"]["notebook_path"] + "_sibling" + }, +}) +PYEDIT + + cp databricks.yml databricks.yml.backup + trace $CLI bundle config-remote-sync --select-ids "jobs:$job_id" --save + trace diff.py databricks.yml.backup databricks.yml + rm databricks.yml.backup +done diff --git a/acceptance/bundle/config-remote-sync/resource_keys/test.toml b/acceptance/bundle/config-remote-sync/resource_keys/test.toml new file mode 100644 index 00000000000..a96be3a4c44 --- /dev/null +++ b/acceptance/bundle/config-remote-sync/resource_keys/test.toml @@ -0,0 +1,4 @@ +RecordRequests = false +Ignore = [".databricks", "databricks.yml", "databricks.yml.backup"] + +Env.DATABRICKS_BUNDLE_ENABLE_EXPERIMENTAL_YAML_SYNC = "true" diff --git a/acceptance/bundle/config-remote-sync/task_rename_revert/databricks.yml.tmpl b/acceptance/bundle/config-remote-sync/task_rename_revert/databricks.yml.tmpl index 7aa0dbecc25..bef94eeca8c 100644 --- a/acceptance/bundle/config-remote-sync/task_rename_revert/databricks.yml.tmpl +++ b/acceptance/bundle/config-remote-sync/task_rename_revert/databricks.yml.tmpl @@ -3,7 +3,7 @@ bundle: resources: jobs: - sample_job: + "sample job's\"quotes": tasks: - task_key: new_task notebook_task: diff --git a/acceptance/bundle/config-remote-sync/task_rename_revert/output.txt b/acceptance/bundle/config-remote-sync/task_rename_revert/output.txt index da1063577ab..87e6aea8afe 100644 --- a/acceptance/bundle/config-remote-sync/task_rename_revert/output.txt +++ b/acceptance/bundle/config-remote-sync/task_rename_revert/output.txt @@ -1,5 +1,5 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... -Created jobs.sample_job +Created jobs.sample job's"quotes Files: 2 uploaded, 0 deleted Resources: 1 created, 0 changed, 0 deleted, 0 unchanged @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged === Sync the rename into config (no redeploy) Detected changes in 1 resource(s): -Resource: resources.jobs.sample_job +Resource: resources.jobs.sample job's"quotes tasks[task_key='new_task']: remove tasks[task_key='new_task_2']: add @@ -18,7 +18,7 @@ Resource: resources.jobs.sample_job --- databricks.yml.backup +++ databricks.yml @@ -6,5 +6,5 @@ - sample_job: + "sample job's\"quotes": tasks: - - task_key: new_task + - task_key: new_task_2 @@ -30,7 +30,7 @@ Resource: resources.jobs.sample_job === Sync the revert into config Detected changes in 1 resource(s): -Resource: resources.jobs.sample_job +Resource: resources.jobs.sample job's"quotes tasks[task_key='new_task']: add tasks[task_key='new_task_2']: remove @@ -40,7 +40,7 @@ Resource: resources.jobs.sample_job --- databricks.yml.backup +++ databricks.yml @@ -6,5 +6,5 @@ - sample_job: + "sample job's\"quotes": tasks: - - task_key: new_task_2 + - task_key: new_task @@ -49,7 +49,7 @@ Resource: resources.jobs.sample_job >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: - delete resources.jobs.sample_job + delete resources.jobs.sample job's"quotes All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default diff --git a/acceptance/bundle/config-remote-sync/task_rename_revert/script b/acceptance/bundle/config-remote-sync/task_rename_revert/script index a01c1196de6..319a6731733 100644 --- a/acceptance/bundle/config-remote-sync/task_rename_revert/script +++ b/acceptance/bundle/config-remote-sync/task_rename_revert/script @@ -8,7 +8,7 @@ cleanup() { trap cleanup EXIT $CLI bundle deploy -job_id="$(read_id.py sample_job)" +job_id="$(read_id.py "sample job's\"quotes")" title "Rename task to new_task_2 remotely" echo diff --git a/bundle/configsync/patch_test.go b/bundle/configsync/patch_test.go index ef0030f95f8..959a4d37195 100644 --- a/bundle/configsync/patch_test.go +++ b/bundle/configsync/patch_test.go @@ -22,7 +22,7 @@ func TestApplyChangesToYAML_PreserveFormatting(t *testing.T) { yamlContent := `# Comment at top resources: jobs: - test_job: + "test.job": name: "Test Job" # Comment before timeout @@ -42,7 +42,7 @@ resources: mutator.DefaultMutators(ctx, b) changes := Changes{ - "resources.jobs.test_job": ResourceChanges{ + "resources.jobs.test.job": ResourceChanges{ "timeout_seconds": &ConfigChangeDesc{ Operation: OperationReplace, Value: 7200, @@ -61,6 +61,7 @@ resources: modified := fileChanges[0].ModifiedContent assert.Contains(t, modified, "# Comment at top") + assert.Contains(t, modified, `"test.job":`) assert.Contains(t, modified, "# Comment before timeout") assert.Contains(t, modified, "timeout_seconds: 7200") assert.Equal(t, 2, strings.Count(modified, "\n\n"), "both blank lines should be preserved") diff --git a/bundle/configsync/resolve.go b/bundle/configsync/resolve.go index cdad5182336..2dc64b275ef 100644 --- a/bundle/configsync/resolve.go +++ b/bundle/configsync/resolve.go @@ -210,6 +210,11 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes for _, resourceKey := range resourceKeys { resourceChanges := configChanges[resourceKey] + // Plan keys are unquoted; only the first two dots separate resource components. + var resourcePath *structpath.PathNode + for _, key := range strings.SplitN(resourceKey, ".", 3) { + resourcePath = structpath.NewStringKey(resourcePath, key) + } fieldPaths := make([]string, 0, len(resourceChanges)) fieldPathsDepths := map[string]int{} @@ -244,13 +249,13 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes // 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 := matchRenamingPairs(b, blocks, resourceKey, resourceChanges) + renames := matchRenamingPairs(b, blocks, resourcePath.String(), resourceChanges) indices := newIndexTracker() for _, fieldPath := range fieldPaths { configChange := resourceChanges[fieldPath] - fullPath := resourceKey + "." + fieldPath + fullPath := resourcePath.String() + "." + fieldPath // The add half of a rename carries no source location of its own; the // pair is written from the remove half, which does. @@ -360,7 +365,7 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes // on it belongs in that same block. filePath = block.file } else { - resourceLocation := b.Config.GetLocation(resourceKey) + resourceLocation := b.Config.GetLocationOf(resourcePath) filePath = resourceLocation.File } if filePath == "" { diff --git a/bundle/configsync/variables.go b/bundle/configsync/variables.go index 2a0ac89cc92..0701bd217b0 100644 --- a/bundle/configsync/variables.go +++ b/bundle/configsync/variables.go @@ -556,10 +556,14 @@ func parseTemplateSegments(template string, resolved dyn.Value) []templateSegmen // preResolvedValueAt returns the pre-resolved dyn.Value at the field path, // if the field exists in the merged pre-resolved config. func preResolvedValueAt(preResolved dyn.Value, fieldPath string) (dyn.Value, bool) { - p, err := dyn.NewPathFromString(fieldPath) + node, err := structpath.ParsePath(fieldPath) if err != nil { return dyn.InvalidValue, false } + p, ok := dyn.FromStructPath(node) + if !ok { + return dyn.InvalidValue, false + } v, err := dyn.GetByPath(preResolved, p) if err != nil { return dyn.InvalidValue, false @@ -603,12 +607,8 @@ func sequenceSiblings(preResolved dyn.Value, fieldPath string) ([]dyn.Value, boo if !hasIndex && !node.BracketStar() { return nil, false } - p, err := dyn.NewPathFromString(node.Parent().String()) - if err != nil { - return nil, false - } - parentValue, err := dyn.GetByPath(preResolved, p) - if err != nil { + parentValue, ok := preResolvedValueAt(preResolved, node.Parent().String()) + if !ok { return nil, false } seq, ok := parentValue.AsSequence()