diff --git a/.nextchanges/bundles/6757.md b/.nextchanges/bundles/6757.md new file mode 100644 index 00000000000..e9d18446299 --- /dev/null +++ b/.nextchanges/bundles/6757.md @@ -0,0 +1 @@ +* direct: Fix a spurious `permissions` update reported on every plan and deploy when a service principal is declared under `user_name` and the Permissions API returns it as `service_principal_name`. Keyed-slice elements in `bundle plan --output json` are now addressed by value as `[='value']` instead of `[field='value']`. ([#6757](https://github.com/databricks/cli/pull/6757)) diff --git a/acceptance/bundle/artifacts/whl_dynamic/out.plan_update.direct.json b/acceptance/bundle/artifacts/whl_dynamic/out.plan_update.direct.json index dd31e902e64..55d732da36b 100644 --- a/acceptance/bundle/artifacts/whl_dynamic/out.plan_update.direct.json +++ b/acceptance/bundle/artifacts/whl_dynamic/out.plan_update.direct.json @@ -159,50 +159,50 @@ "reason": "empty", "remote": {} }, - "environments[environment_key='test_env'].spec.dependencies[0]": { + "environments[='test_env'].spec.dependencies[0]": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl" }, - "tasks[task_key='ServerlessTestTask'].email_notifications": { + "tasks[='ServerlessTestTask'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='ServerlessTestTask'].run_if": { + "tasks[='ServerlessTestTask'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='ServerlessTestTask'].timeout_seconds": { + "tasks[='ServerlessTestTask'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='TestTask'].email_notifications": { + "tasks[='TestTask'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='TestTask'].for_each_task.task.libraries[0].whl": { + "tasks[='TestTask'].for_each_task.task.libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl" }, - "tasks[task_key='TestTask'].libraries[0].whl": { + "tasks[='TestTask'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl" }, - "tasks[task_key='TestTask'].run_if": { + "tasks[='TestTask'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='TestTask'].timeout_seconds": { + "tasks[='TestTask'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/config-remote-sync/cluster_policy_remote_addition/output.txt b/acceptance/bundle/config-remote-sync/cluster_policy_remote_addition/output.txt index 015028434a2..e5a315dc577 100644 --- a/acceptance/bundle/config-remote-sync/cluster_policy_remote_addition/output.txt +++ b/acceptance/bundle/config-remote-sync/cluster_policy_remote_addition/output.txt @@ -10,7 +10,7 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.test_job - tasks[task_key='test'].new_cluster.spark_conf: add + tasks[='test'].new_cluster.spark_conf: add diff --git a/acceptance/bundle/config-remote-sync/job_fields/output.txt b/acceptance/bundle/config-remote-sync/job_fields/output.txt index 3f7d3f1b14c..16879650ba0 100644 --- a/acceptance/bundle/config-remote-sync/job_fields/output.txt +++ b/acceptance/bundle/config-remote-sync/job_fields/output.txt @@ -12,13 +12,13 @@ Detected changes in 1 resource(s): Resource: resources.jobs.my_job email_notifications.no_alert_for_skipped_runs: add email_notifications.on_failure: add - job_clusters[job_cluster_key='test_cluster']: remove - job_clusters[job_cluster_key='test_cluster_renamed']: add + job_clusters[='test_cluster']: remove + job_clusters[='test_cluster_renamed']: add name: replace - parameters[name='region']: add + parameters[='region']: add tags['team']: add - tasks[task_key='inline_cluster_task'].new_cluster.num_workers: replace - tasks[task_key='main'].job_cluster_key: replace + tasks[='inline_cluster_task'].new_cluster.num_workers: replace + tasks[='main'].job_cluster_key: replace trigger.pause_status: add trigger.periodic: remove trigger.table_update: add 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 e009aa87c0d..dd79c2690a4 100644 --- a/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt +++ b/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt @@ -10,12 +10,12 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 2 resource(s): Resource: resources.jobs.my_job - tasks[task_key='c_task'].depends_on[task_key='b_task']: add - tasks[task_key='c_task'].depends_on[task_key='d_task']: remove - tasks[task_key='c_task'].new_cluster.num_workers: replace - tasks[task_key='c_task'].timeout_seconds: add - tasks[task_key='d_task']: remove - tasks[task_key='e_task']: add + tasks[='c_task'].depends_on[='b_task']: add + tasks[='c_task'].depends_on[='d_task']: remove + tasks[='c_task'].new_cluster.num_workers: replace + tasks[='c_task'].timeout_seconds: add + tasks[='d_task']: remove + tasks[='e_task']: add Resource: resources.jobs.no_tasks_job tasks: add @@ -81,14 +81,14 @@ Resource: resources.jobs.no_tasks_job Detected changes in 1 resource(s): Resource: resources.jobs.rename_task_job - tasks[task_key='a_task'].notebook_task.notebook_path: replace - tasks[task_key='b_task']: remove - tasks[task_key='b_task_renamed']: add - tasks[task_key='c_task'].depends_on[task_key='b_task']: remove - tasks[task_key='c_task'].depends_on[task_key='b_task_renamed']: add - tasks[task_key='d_task'].depends_on[task_key='b_task']: remove - tasks[task_key='d_task'].depends_on[task_key='b_task_renamed']: add - tasks[task_key='synced_task']: add + tasks[='a_task'].notebook_task.notebook_path: replace + tasks[='b_task']: remove + tasks[='b_task_renamed']: add + tasks[='c_task'].depends_on[='b_task']: remove + tasks[='c_task'].depends_on[='b_task_renamed']: add + tasks[='d_task'].depends_on[='b_task']: remove + tasks[='d_task'].depends_on[='b_task_renamed']: add + tasks[='synced_task']: add diff --git a/acceptance/bundle/config-remote-sync/job_params_variables/output.txt b/acceptance/bundle/config-remote-sync/job_params_variables/output.txt index 543d48d2371..941b6c3548b 100644 --- a/acceptance/bundle/config-remote-sync/job_params_variables/output.txt +++ b/acceptance/bundle/config-remote-sync/job_params_variables/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.my_job - parameters[name='region']: add + parameters[='region']: add diff --git a/acceptance/bundle/config-remote-sync/job_pipeline_task/output.txt b/acceptance/bundle/config-remote-sync/job_pipeline_task/output.txt index 539987111fc..398c0c9b121 100644 --- a/acceptance/bundle/config-remote-sync/job_pipeline_task/output.txt +++ b/acceptance/bundle/config-remote-sync/job_pipeline_task/output.txt @@ -10,7 +10,7 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 2 resource(s): Resource: resources.jobs.my_job - tasks[task_key='run_pipeline'].pipeline_task.full_refresh: replace + tasks[='run_pipeline'].pipeline_task.full_refresh: replace Resource: resources.pipelines.my_pipeline continuous: add diff --git a/acceptance/bundle/config-remote-sync/multiple_files/output.txt b/acceptance/bundle/config-remote-sync/multiple_files/output.txt index a2a46515664..c3c76425674 100644 --- a/acceptance/bundle/config-remote-sync/multiple_files/output.txt +++ b/acceptance/bundle/config-remote-sync/multiple_files/output.txt @@ -13,18 +13,18 @@ Detected changes in 2 resource(s): Resource: resources.jobs.job_one max_concurrent_runs: replace - tasks[task_key='a_task'].depends_on[task_key='c_task']: remove - tasks[task_key='a_task'].depends_on[task_key='c_task_renamed']: add - tasks[task_key='c_task']: remove - tasks[task_key='c_task_renamed']: add - tasks[task_key='synced_task']: add + tasks[='a_task'].depends_on[='c_task']: remove + tasks[='a_task'].depends_on[='c_task_renamed']: add + tasks[='c_task']: remove + tasks[='c_task_renamed']: add + tasks[='synced_task']: add Resource: resources.jobs.job_two max_concurrent_runs: replace - tasks[task_key='extra_task']: replace - tasks[task_key='extra_task'].new_cluster: add - tasks[task_key='run_pipeline']: remove - tasks[task_key='run_pipeline_renamed']: add + tasks[='extra_task']: replace + tasks[='extra_task'].new_cluster: add + tasks[='run_pipeline']: remove + tasks[='run_pipeline_renamed']: add diff --git a/acceptance/bundle/config-remote-sync/resolve_variables/output.txt b/acceptance/bundle/config-remote-sync/resolve_variables/output.txt index 98c0ecb9869..acdc9bdafb3 100644 --- a/acceptance/bundle/config-remote-sync/resolve_variables/output.txt +++ b/acceptance/bundle/config-remote-sync/resolve_variables/output.txt @@ -10,22 +10,22 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 2 resource(s): Resource: resources.jobs.my_job - environments[environment_key='default'].spec.dependencies: replace - environments[environment_key='secondary']: add - parameters[name='catalog'].default: replace - parameters[name='data_catalog']: add - parameters[name='deploy_env']: add - parameters[name='deploy_target']: add - parameters[name='env'].default: replace - parameters[name='file_sourced']: add - parameters[name='region']: add - parameters[name='some_schema']: add + environments[='default'].spec.dependencies: replace + environments[='secondary']: add + parameters[='catalog'].default: replace + parameters[='data_catalog']: add + parameters[='deploy_env']: add + parameters[='deploy_target']: add + parameters[='env'].default: replace + parameters[='file_sourced']: add + parameters[='region']: add + parameters[='some_schema']: add tags['deployment']: add tags['dev']: remove - tasks[task_key='main'].notebook_task.base_parameters['source_path']: replace - tasks[task_key='run_pipeline'].pipeline_task.full_refresh: replace - tasks[task_key='run_pipeline_again']: add - tasks[task_key='secondary']: add + tasks[='main'].notebook_task.base_parameters['source_path']: replace + tasks[='run_pipeline'].pipeline_task.full_refresh: replace + tasks[='run_pipeline_again']: add + tasks[='secondary']: add Resource: resources.pipelines.my_pipeline environment.dependencies: replace diff --git a/acceptance/bundle/config-remote-sync/split/cli_default_split_element/output.txt b/acceptance/bundle/config-remote-sync/split/cli_default_split_element/output.txt index ace5341ac66..f5c890ff8e3 100644 --- a/acceptance/bundle/config-remote-sync/split/cli_default_split_element/output.txt +++ b/acceptance/bundle/config-remote-sync/split/cli_default_split_element/output.txt @@ -12,7 +12,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.cli_default_job - tasks[task_key='shared'].existing_cluster_id: add + tasks[='shared'].existing_cluster_id: add diff --git a/acceptance/bundle/config-remote-sync/split/dotted_target/output.txt b/acceptance/bundle/config-remote-sync/split/dotted_target/output.txt index ffdc480eb74..4e5c08a90e2 100644 --- a/acceptance/bundle/config-remote-sync/split/dotted_target/output.txt +++ b/acceptance/bundle/config-remote-sync/split/dotted_target/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.dotted_job - tasks[task_key='main'].timeout_seconds: replace + tasks[='main'].timeout_seconds: replace diff --git a/acceptance/bundle/config-remote-sync/split/isolation/output.txt b/acceptance/bundle/config-remote-sync/split/isolation/output.txt index 0d2b8d4a27f..c3e60758369 100644 --- a/acceptance/bundle/config-remote-sync/split/isolation/output.txt +++ b/acceptance/bundle/config-remote-sync/split/isolation/output.txt @@ -9,8 +9,8 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 2 resource(s): Resource: resources.jobs.job_a - tasks[task_key='shared']: remove - tasks[task_key='shared_renamed']: add + tasks[='shared']: remove + tasks[='shared_renamed']: add Resource: resources.jobs.job_b max_concurrent_runs: replace diff --git a/acceptance/bundle/config-remote-sync/split/keyed_edit/output.txt b/acceptance/bundle/config-remote-sync/split/keyed_edit/output.txt index fb5a0512731..a518cdb6fd9 100644 --- a/acceptance/bundle/config-remote-sync/split/keyed_edit/output.txt +++ b/acceptance/bundle/config-remote-sync/split/keyed_edit/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.split_job - tasks[task_key='alpha'].timeout_seconds: add + tasks[='alpha'].timeout_seconds: add @@ -27,7 +27,7 @@ Resource: resources.jobs.split_job Detected changes in 1 resource(s): Resource: resources.jobs.split_job - tasks[task_key='mu'].timeout_seconds: add + tasks[='mu'].timeout_seconds: add diff --git a/acceptance/bundle/config-remote-sync/split/keyed_remove/output.txt b/acceptance/bundle/config-remote-sync/split/keyed_remove/output.txt index 619a1dca0f3..3668d7db502 100644 --- a/acceptance/bundle/config-remote-sync/split/keyed_remove/output.txt +++ b/acceptance/bundle/config-remote-sync/split/keyed_remove/output.txt @@ -9,8 +9,8 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.remove_job - tasks[task_key='alpha']: remove - tasks[task_key='gamma']: remove + tasks[='alpha']: remove + tasks[='gamma']: remove @@ -43,7 +43,7 @@ Resource: resources.jobs.remove_job Detected changes in 1 resource(s): Resource: resources.jobs.twoblock_remove_job - tasks[task_key='both']: remove + tasks[='both']: remove diff --git a/acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt b/acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt index 39c95b0a242..c951c223ee9 100644 --- a/acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt +++ b/acceptance/bundle/config-remote-sync/split/keyed_rename/output.txt @@ -9,10 +9,10 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged 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 + tasks[='a_shared']: add + tasks[='shared']: remove + tasks[='solo']: remove + tasks[='z_solo']: add @@ -50,9 +50,9 @@ Resource: resources.jobs.rename_job 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 + tasks[='aaa']: remove + tasks[='mmm']: remove + tasks[='mmm2']: add @@ -79,9 +79,9 @@ Resource: resources.jobs.shift_job 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 + tasks[='a_shared']: remove + tasks[='b_shared']: add + tasks[='z_solo']: replace diff --git a/acceptance/bundle/config-remote-sync/split/keyed_twoblock/output.txt b/acceptance/bundle/config-remote-sync/split/keyed_twoblock/output.txt index 36fdca3235d..29e549395d6 100644 --- a/acceptance/bundle/config-remote-sync/split/keyed_twoblock/output.txt +++ b/acceptance/bundle/config-remote-sync/split/keyed_twoblock/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.twoblock_job - tasks[task_key='omega'].timeout_seconds: add + tasks[='omega'].timeout_seconds: add @@ -27,9 +27,9 @@ Resource: resources.jobs.twoblock_job Detected changes in 1 resource(s): Resource: resources.jobs.twoblock_job - tasks[task_key='shared'].max_retries: replace - tasks[task_key='shared'].min_retry_interval_millis: add - tasks[task_key='shared'].timeout_seconds: replace + tasks[='shared'].max_retries: replace + tasks[='shared'].min_retry_interval_millis: add + tasks[='shared'].timeout_seconds: replace diff --git a/acceptance/bundle/config-remote-sync/split/multifile/output.txt b/acceptance/bundle/config-remote-sync/split/multifile/output.txt index a8a7da3874a..f22a2ae942b 100644 --- a/acceptance/bundle/config-remote-sync/split/multifile/output.txt +++ b/acceptance/bundle/config-remote-sync/split/multifile/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.multifile_job - tasks[task_key='from_second_file'].timeout_seconds: add + tasks[='from_second_file'].timeout_seconds: add @@ -37,8 +37,8 @@ Resource: resources.jobs.multifile_job Detected changes in 1 resource(s): Resource: resources.jobs.multifile_job - tasks[task_key='in_both_files'].max_retries: replace - tasks[task_key='in_both_files'].timeout_seconds: replace + tasks[='in_both_files'].max_retries: replace + tasks[='in_both_files'].timeout_seconds: replace @@ -71,7 +71,7 @@ Resource: resources.jobs.multifile_job Detected changes in 1 resource(s): Resource: resources.jobs.multifile_job - tasks[task_key='same_field_both_files'].max_retries: replace + tasks[='same_field_both_files'].max_retries: replace diff --git a/acceptance/bundle/config-remote-sync/split/nested_add_split_parent/output.txt b/acceptance/bundle/config-remote-sync/split/nested_add_split_parent/output.txt index 368b97e457e..8a3e45a0b02 100644 --- a/acceptance/bundle/config-remote-sync/split/nested_add_split_parent/output.txt +++ b/acceptance/bundle/config-remote-sync/split/nested_add_split_parent/output.txt @@ -8,8 +8,8 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.nested_add_job - tasks[task_key='mid_new']: add - tasks[task_key='zzz_shared'].depends_on[task_key='mid_new']: add + tasks[='mid_new']: add + tasks[='zzz_shared'].depends_on[='mid_new']: add diff --git a/acceptance/bundle/config-remote-sync/split/nested_sequence/output.txt b/acceptance/bundle/config-remote-sync/split/nested_sequence/output.txt index 9bd5f51ce28..2bfd6cb977d 100644 --- a/acceptance/bundle/config-remote-sync/split/nested_sequence/output.txt +++ b/acceptance/bundle/config-remote-sync/split/nested_sequence/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.nested_job - tasks[task_key='second'].libraries: replace + tasks[='second'].libraries: replace diff --git a/acceptance/bundle/config-remote-sync/split/remove_field_both_blocks/output.txt b/acceptance/bundle/config-remote-sync/split/remove_field_both_blocks/output.txt index 03611d23f1a..05ebe61f4aa 100644 --- a/acceptance/bundle/config-remote-sync/split/remove_field_both_blocks/output.txt +++ b/acceptance/bundle/config-remote-sync/split/remove_field_both_blocks/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.shared_field_job - tasks[task_key='shared'].max_retries: remove + tasks[='shared'].max_retries: remove diff --git a/acceptance/bundle/config-remote-sync/split/remove_with_unrelated_add/output.txt b/acceptance/bundle/config-remote-sync/split/remove_with_unrelated_add/output.txt index 58f70cfc3ca..f77bfd98e1d 100644 --- a/acceptance/bundle/config-remote-sync/split/remove_with_unrelated_add/output.txt +++ b/acceptance/bundle/config-remote-sync/split/remove_with_unrelated_add/output.txt @@ -8,8 +8,8 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.unrelated_add_job - tasks[task_key='brand_new']: add - tasks[task_key='shared']: remove + tasks[='brand_new']: add + tasks[='shared']: remove diff --git a/acceptance/bundle/config-remote-sync/split/rename_ambiguous_pairing/output.txt b/acceptance/bundle/config-remote-sync/split/rename_ambiguous_pairing/output.txt index 737a1c9c706..b4f28accff0 100644 --- a/acceptance/bundle/config-remote-sync/split/rename_ambiguous_pairing/output.txt +++ b/acceptance/bundle/config-remote-sync/split/rename_ambiguous_pairing/output.txt @@ -8,10 +8,10 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.ambiguous_rename_job - tasks[task_key='aaa']: add - tasks[task_key='alpha']: remove - tasks[task_key='beta']: remove - tasks[task_key='zzz']: add + tasks[='aaa']: add + tasks[='alpha']: remove + tasks[='beta']: remove + tasks[='zzz']: add diff --git a/acceptance/bundle/config-remote-sync/split/rename_ambiguous_single_block/output.txt b/acceptance/bundle/config-remote-sync/split/rename_ambiguous_single_block/output.txt index 8871cb0170c..7e553b800b9 100644 --- a/acceptance/bundle/config-remote-sync/split/rename_ambiguous_single_block/output.txt +++ b/acceptance/bundle/config-remote-sync/split/rename_ambiguous_single_block/output.txt @@ -8,14 +8,14 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.ambiguous_single_block_job - tasks[task_key='aaa']: add - tasks[task_key='alpha']: remove - tasks[task_key='beta']: remove - tasks[task_key='gamma'].depends_on[task_key='aaa']: add - tasks[task_key='gamma'].depends_on[task_key='alpha']: remove - tasks[task_key='gamma'].depends_on[task_key='beta']: remove - tasks[task_key='gamma'].depends_on[task_key='zzz']: add - tasks[task_key='zzz']: add + tasks[='aaa']: add + tasks[='alpha']: remove + tasks[='beta']: remove + tasks[='gamma'].depends_on[='aaa']: add + tasks[='gamma'].depends_on[='alpha']: remove + tasks[='gamma'].depends_on[='beta']: remove + tasks[='gamma'].depends_on[='zzz']: add + tasks[='zzz']: add diff --git a/acceptance/bundle/config-remote-sync/split/rename_two_removes_one_add/output.txt b/acceptance/bundle/config-remote-sync/split/rename_two_removes_one_add/output.txt index 42433f62e84..ed8cc14335b 100644 --- a/acceptance/bundle/config-remote-sync/split/rename_two_removes_one_add/output.txt +++ b/acceptance/bundle/config-remote-sync/split/rename_two_removes_one_add/output.txt @@ -8,9 +8,9 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.two_removes_job - tasks[task_key='task_renamed']: add - tasks[task_key='task_target']: remove - tasks[task_key='task_toplevel']: remove + tasks[='task_renamed']: add + tasks[='task_target']: remove + tasks[='task_toplevel']: remove diff --git a/acceptance/bundle/config-remote-sync/split/target_variable/output.txt b/acceptance/bundle/config-remote-sync/split/target_variable/output.txt index bf1db14790d..e98e2fd5ec7 100644 --- a/acceptance/bundle/config-remote-sync/split/target_variable/output.txt +++ b/acceptance/bundle/config-remote-sync/split/target_variable/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.var_job - tasks[task_key='zzz'].notebook_task.notebook_path: replace + tasks[='zzz'].notebook_task.notebook_path: replace diff --git a/acceptance/bundle/config-remote-sync/split/variable_file_order/output.txt b/acceptance/bundle/config-remote-sync/split/variable_file_order/output.txt index 4db5423733e..733f964804d 100644 --- a/acceptance/bundle/config-remote-sync/split/variable_file_order/output.txt +++ b/acceptance/bundle/config-remote-sync/split/variable_file_order/output.txt @@ -8,7 +8,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.file_order_job - tasks[task_key='aaa'].notebook_task.notebook_path: replace + tasks[='aaa'].notebook_task.notebook_path: replace 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 2ebbe21da7a..5d324cff1c9 100644 --- a/acceptance/bundle/config-remote-sync/task_rename_revert/output.txt +++ b/acceptance/bundle/config-remote-sync/task_rename_revert/output.txt @@ -9,8 +9,8 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged Detected changes in 1 resource(s): Resource: resources.jobs.sample_job - tasks[task_key='new_task']: remove - tasks[task_key='new_task_2']: add + tasks[='new_task']: remove + tasks[='new_task_2']: add @@ -31,8 +31,8 @@ Resource: resources.jobs.sample_job Detected changes in 1 resource(s): Resource: resources.jobs.sample_job - tasks[task_key='new_task']: add - tasks[task_key='new_task_2']: remove + tasks[='new_task']: add + tasks[='new_task_2']: remove diff --git a/acceptance/bundle/config-remote-sync/validation_errors/output.txt b/acceptance/bundle/config-remote-sync/validation_errors/output.txt index 867651f8fc1..155b34089aa 100644 --- a/acceptance/bundle/config-remote-sync/validation_errors/output.txt +++ b/acceptance/bundle/config-remote-sync/validation_errors/output.txt @@ -11,7 +11,7 @@ Detected changes in 2 resource(s): Resource: resources.jobs.my_job git_source: add - tasks[task_key='main'].notebook_task.notebook_path: replace + tasks[='main'].notebook_task.notebook_path: replace Resource: resources.pipelines.my_pipeline root_path: replace diff --git a/acceptance/bundle/config-remote-sync/variable_reference_parent/output.txt b/acceptance/bundle/config-remote-sync/variable_reference_parent/output.txt index 0ad1137275f..1dd0cc8a183 100644 --- a/acceptance/bundle/config-remote-sync/variable_reference_parent/output.txt +++ b/acceptance/bundle/config-remote-sync/variable_reference_parent/output.txt @@ -11,7 +11,7 @@ written into that scalar node; the change is skipped while unrelated edits still Detected changes in 1 resource(s): Resource: resources.jobs.my_job - job_clusters[job_cluster_key='shared'].new_cluster.spark_conf['spark.executor.memory']: skip + job_clusters[='shared'].new_cluster.spark_conf['spark.executor.memory']: skip max_concurrent_runs: replace diff --git a/acceptance/bundle/deploy/readplan/grants-remove-principal/out.plan.grants.json b/acceptance/bundle/deploy/readplan/grants-remove-principal/out.plan.grants.json index 0b90f0b19e6..d63a8572f4d 100644 --- a/acceptance/bundle/deploy/readplan/grants-remove-principal/out.plan.grants.json +++ b/acceptance/bundle/deploy/readplan/grants-remove-principal/out.plan.grants.json @@ -39,7 +39,7 @@ ] }, "changes": { - "[principal='extra@example.test']": { + "[='extra@example.test']": { "action": "update", "old": { "principal": "extra@example.test", diff --git a/acceptance/bundle/migrate/basic/out.plan_update.json b/acceptance/bundle/migrate/basic/out.plan_update.json index 02aa38f21c5..1bda8a65f90 100644 --- a/acceptance/bundle/migrate/basic/out.plan_update.json +++ b/acceptance/bundle/migrate/basic/out.plan_update.json @@ -73,22 +73,22 @@ "new": "Test Migrated Job", "remote": "Test Migration Job" }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].notebook_task.source": { + "tasks[='main'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/migrate/default-python/out.plan_after_deploy.json b/acceptance/bundle/migrate/default-python/out.plan_after_deploy.json index 2ec4018e820..02a4548688a 100644 --- a/acceptance/bundle/migrate/default-python/out.plan_after_deploy.json +++ b/acceptance/bundle/migrate/default-python/out.plan_after_deploy.json @@ -229,64 +229,64 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].libraries[0].whl": { + "tasks[='notebook_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl" }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].libraries[0].whl": { + "tasks[='python_wheel_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl" }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/migrate/default-python/out.plan_after_migration.json b/acceptance/bundle/migrate/default-python/out.plan_after_migration.json index de570cc7ade..6d1d7daabc9 100644 --- a/acceptance/bundle/migrate/default-python/out.plan_after_migration.json +++ b/acceptance/bundle/migrate/default-python/out.plan_after_migration.json @@ -229,64 +229,64 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].libraries[0].whl": { + "tasks[='notebook_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl" }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].libraries[0].whl": { + "tasks[='python_wheel_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl" }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/migrate/default-python/output.txt b/acceptance/bundle/migrate/default-python/output.txt index 46597c8462e..9936de3c49f 100644 --- a/acceptance/bundle/migrate/default-python/output.txt +++ b/acceptance/bundle/migrate/default-python/output.txt @@ -71,64 +71,64 @@ Building python_artifact... "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].libraries[0].whl": { + "tasks[='notebook_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl" }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].libraries[0].whl": { + "tasks[='python_wheel_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl" }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 @@ -190,64 +190,64 @@ Building python_artifact... "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].libraries[0].whl": { + "tasks[='notebook_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl" }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].libraries[0].whl": { + "tasks[='python_wheel_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][2]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl" }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resource_deps/jobs_update_remote/out.plan_update.direct.json b/acceptance/bundle/resource_deps/jobs_update_remote/out.plan_update.direct.json index 5f91a583afe..9a2b672acb2 100644 --- a/acceptance/bundle/resource_deps/jobs_update_remote/out.plan_update.direct.json +++ b/acceptance/bundle/resource_deps/jobs_update_remote/out.plan_update.direct.json @@ -125,7 +125,7 @@ "reason": "empty", "remote": {} }, - "job_clusters[job_cluster_key='key'].new_cluster.num_workers": { + "job_clusters[='key'].new_cluster.num_workers": { "action": "skip", "reason": "empty", "old": 0, diff --git a/acceptance/bundle/resources/grants/catalogs/out.plan2.direct.json b/acceptance/bundle/resources/grants/catalogs/out.plan2.direct.json index 92d227bc232..e7b9ee796f2 100644 --- a/acceptance/bundle/resources/grants/catalogs/out.plan2.direct.json +++ b/acceptance/bundle/resources/grants/catalogs/out.plan2.direct.json @@ -49,13 +49,13 @@ ] }, "changes": { - "[principal='deco-test-user@databricks.com'].privileges[0]": { + "[='deco-test-user@databricks.com'].privileges[0]": { "action": "update", "old": "CREATE_SCHEMA", "new": "USE_CATALOG", "remote": "CREATE_SCHEMA" }, - "[principal='deco-test-user@databricks.com'].privileges[1]": { + "[='deco-test-user@databricks.com'].privileges[1]": { "action": "update", "old": "USE_CATALOG", "new": "USE_SCHEMA", diff --git a/acceptance/bundle/resources/grants/schemas/change_privilege/out.plan2.direct.json b/acceptance/bundle/resources/grants/schemas/change_privilege/out.plan2.direct.json index 5a9c37ee3f1..7e5a9e103fb 100644 --- a/acceptance/bundle/resources/grants/schemas/change_privilege/out.plan2.direct.json +++ b/acceptance/bundle/resources/grants/schemas/change_privilege/out.plan2.direct.json @@ -54,13 +54,13 @@ ] }, "changes": { - "[principal='deco-test-user@databricks.com'].privileges[0]": { + "[='deco-test-user@databricks.com'].privileges[0]": { "action": "update", "old": "CREATE_TABLE", "new": "APPLY_TAG", "remote": "CREATE_TABLE" }, - "[principal='deco-test-user@databricks.com'].privileges[1]": { + "[='deco-test-user@databricks.com'].privileges[1]": { "action": "update", "old": "USE_SCHEMA", "new": "CREATE_TABLE", diff --git a/acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan.direct.json b/acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan.direct.json index 038a187a8ed..61aafcc6956 100644 --- a/acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan.direct.json +++ b/acceptance/bundle/resources/grants/schemas/out_of_band_principal/out.plan.direct.json @@ -47,7 +47,7 @@ "full_name": "main.schema_out_of_band_principal_[UNIQUE_NAME]" }, "changes": { - "[principal='deco-test-user@databricks.com']": { + "[='deco-test-user@databricks.com']": { "action": "update", "remote": { "principal": "deco-test-user@databricks.com", diff --git a/acceptance/bundle/resources/grants/schemas/remove_principal_manage/out.plan.changes.direct.json b/acceptance/bundle/resources/grants/schemas/remove_principal_manage/out.plan.changes.direct.json index b88d41e1a1d..528506f773c 100644 --- a/acceptance/bundle/resources/grants/schemas/remove_principal_manage/out.plan.changes.direct.json +++ b/acceptance/bundle/resources/grants/schemas/remove_principal_manage/out.plan.changes.direct.json @@ -1,5 +1,5 @@ { - "[principal='deco-test-user@databricks.com']": { + "[='deco-test-user@databricks.com']": { "action": "update", "old": { "principal": "deco-test-user@databricks.com", diff --git a/acceptance/bundle/resources/grants/volumes/out.plan2.direct.json b/acceptance/bundle/resources/grants/volumes/out.plan2.direct.json index d6a7b862722..cb4a22c0629 100644 --- a/acceptance/bundle/resources/grants/volumes/out.plan2.direct.json +++ b/acceptance/bundle/resources/grants/volumes/out.plan2.direct.json @@ -81,13 +81,13 @@ ] }, "changes": { - "[principal='deco-test-user@databricks.com'].privileges[0]": { + "[='deco-test-user@databricks.com'].privileges[0]": { "action": "update", "old": "READ_VOLUME", "new": "MANAGE", "remote": "READ_VOLUME" }, - "[principal='deco-test-user@databricks.com'].privileges[1]": { + "[='deco-test-user@databricks.com'].privileges[1]": { "action": "update", "old": "WRITE_VOLUME", "new": "READ_VOLUME", diff --git a/acceptance/bundle/resources/jobs/cluster_policy/fixed_addition/output.txt b/acceptance/bundle/resources/jobs/cluster_policy/fixed_addition/output.txt index 52e33b9bf3e..327205fee4d 100644 --- a/acceptance/bundle/resources/jobs/cluster_policy/fixed_addition/output.txt +++ b/acceptance/bundle/resources/jobs/cluster_policy/fixed_addition/output.txt @@ -15,38 +15,38 @@ Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged >>> [CLI] bundle plan -o json { - "job_clusters[job_cluster_key='job_cluster'].new_cluster.custom_tags": { + "job_clusters[='job_cluster'].new_cluster.custom_tags": { "action": "skip", "reason": "remote_addition", "remote": { "CostCenter": "from-policy" } }, - "job_clusters[job_cluster_key='job_cluster'].new_cluster.spark_version": { + "job_clusters[='job_cluster'].new_cluster.spark_version": { "action": "skip", "reason": "remote_addition", "remote": "13.3.x-snapshot-scala2.12" }, - "tasks[task_key='for_each'].for_each_task.task.new_cluster.custom_tags": { + "tasks[='for_each'].for_each_task.task.new_cluster.custom_tags": { "action": "skip", "reason": "remote_addition", "remote": { "CostCenter": "from-policy" } }, - "tasks[task_key='for_each'].for_each_task.task.new_cluster.spark_version": { + "tasks[='for_each'].for_each_task.task.new_cluster.spark_version": { "action": "skip", "reason": "remote_addition", "remote": "13.3.x-snapshot-scala2.12" }, - "tasks[task_key='task_cluster'].new_cluster.custom_tags": { + "tasks[='task_cluster'].new_cluster.custom_tags": { "action": "skip", "reason": "remote_addition", "remote": { "CostCenter": "from-policy" } }, - "tasks[task_key='task_cluster'].new_cluster.spark_version": { + "tasks[='task_cluster'].new_cluster.spark_version": { "action": "skip", "reason": "remote_addition", "remote": "13.3.x-snapshot-scala2.12" diff --git a/acceptance/bundle/resources/jobs/cluster_policy/owned_tag_drift/output.txt b/acceptance/bundle/resources/jobs/cluster_policy/owned_tag_drift/output.txt index f49402a4dd9..f06afa68045 100644 --- a/acceptance/bundle/resources/jobs/cluster_policy/owned_tag_drift/output.txt +++ b/acceptance/bundle/resources/jobs/cluster_policy/owned_tag_drift/output.txt @@ -19,17 +19,17 @@ Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged >>> [CLI] bundle plan -o json { - "job_clusters[job_cluster_key='main'].new_cluster.custom_tags['CostCenter']": { + "job_clusters[='main'].new_cluster.custom_tags['CostCenter']": { "action": "skip", "reason": "remote_addition", "remote": "from-policy" }, - "job_clusters[job_cluster_key='main'].new_cluster.custom_tags['Mine']": { + "job_clusters[='main'].new_cluster.custom_tags['Mine']": { "action": "update", "old": "mine", "remote": "changed-out-of-band" }, - "job_clusters[job_cluster_key='main'].new_cluster.custom_tags['Other']": { + "job_clusters[='main'].new_cluster.custom_tags['Other']": { "action": "update", "new": "other" } diff --git a/acceptance/bundle/resources/jobs/cluster_policy/policy_id_scoping/output.txt b/acceptance/bundle/resources/jobs/cluster_policy/policy_id_scoping/output.txt index 6ed5b94d741..54422f377b2 100644 --- a/acceptance/bundle/resources/jobs/cluster_policy/policy_id_scoping/output.txt +++ b/acceptance/bundle/resources/jobs/cluster_policy/policy_id_scoping/output.txt @@ -12,14 +12,14 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged >>> [CLI] bundle plan -o json { - "job_clusters[job_cluster_key='gated'].new_cluster.custom_tags": { + "job_clusters[='gated'].new_cluster.custom_tags": { "action": "skip", "reason": "remote_addition", "remote": { "CostCenter": "from-policy" } }, - "job_clusters[job_cluster_key='ungated'].new_cluster.custom_tags": { + "job_clusters[='ungated'].new_cluster.custom_tags": { "action": "update", "remote": { "Injected": "out-of-band" diff --git a/acceptance/bundle/resources/jobs/delete_task/out.plan_update.direct.json b/acceptance/bundle/resources/jobs/delete_task/out.plan_update.direct.json index 86b91fc8ded..bb2068f5187 100644 --- a/acceptance/bundle/resources/jobs/delete_task/out.plan_update.direct.json +++ b/acceptance/bundle/resources/jobs/delete_task/out.plan_update.direct.json @@ -108,7 +108,7 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='TestTask1']": { + "tasks[='TestTask1']": { "action": "update", "old": { "existing_cluster_id": "0717-132531-5opeqon1", @@ -140,17 +140,17 @@ "timeout_seconds": 0 } }, - "tasks[task_key='TestTask2'].email_notifications": { + "tasks[='TestTask2'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='TestTask2'].run_if": { + "tasks[='TestTask2'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='TestTask2'].timeout_seconds": { + "tasks[='TestTask2'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/jobs/on_failure_empty_slice/out.plan.direct.json b/acceptance/bundle/resources/jobs/on_failure_empty_slice/out.plan.direct.json index 816bda2d675..7ec14390721 100644 --- a/acceptance/bundle/resources/jobs/on_failure_empty_slice/out.plan.direct.json +++ b/acceptance/bundle/resources/jobs/on_failure_empty_slice/out.plan.direct.json @@ -4,22 +4,22 @@ "reason": "empty", "new": [] }, - "tasks[task_key='usage_logs_grants'].email_notifications": { + "tasks[='usage_logs_grants'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='usage_logs_grants'].notebook_task.source": { + "tasks[='usage_logs_grants'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='usage_logs_grants'].run_if": { + "tasks[='usage_logs_grants'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='usage_logs_grants'].timeout_seconds": { + "tasks[='usage_logs_grants'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/jobs/remote_add_tag/out.plan_post_update.direct.json b/acceptance/bundle/resources/jobs/remote_add_tag/out.plan_post_update.direct.json index d52ca412cce..bb622b2a3d1 100644 --- a/acceptance/bundle/resources/jobs/remote_add_tag/out.plan_post_update.direct.json +++ b/acceptance/bundle/resources/jobs/remote_add_tag/out.plan_post_update.direct.json @@ -91,17 +91,17 @@ "action": "update", "remote": "new_value" }, - "tasks[task_key='TestTask'].email_notifications": { + "tasks[='TestTask'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='TestTask'].run_if": { + "tasks[='TestTask'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='TestTask'].timeout_seconds": { + "tasks[='TestTask'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/jobs/remote_matches_config/out.plan.direct.json b/acceptance/bundle/resources/jobs/remote_matches_config/out.plan.direct.json index a1c295b4715..de781bdca62 100644 --- a/acceptance/bundle/resources/jobs/remote_matches_config/out.plan.direct.json +++ b/acceptance/bundle/resources/jobs/remote_matches_config/out.plan.direct.json @@ -65,12 +65,12 @@ "new": 2, "remote": 2 }, - "tasks[task_key='main_task'].email_notifications": { + "tasks[='main_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main_task'].new_cluster.aws_attributes": { + "tasks[='main_task'].new_cluster.aws_attributes": { "action": "skip", "reason": "managed", "remote": { @@ -78,27 +78,27 @@ "zone_id": "us-east-1c" } }, - "tasks[task_key='main_task'].new_cluster.data_security_mode": { + "tasks[='main_task'].new_cluster.data_security_mode": { "action": "skip", "reason": "backend_default", "remote": "SINGLE_USER" }, - "tasks[task_key='main_task'].new_cluster.enable_elastic_disk": { + "tasks[='main_task'].new_cluster.enable_elastic_disk": { "action": "skip", "reason": "empty", "remote": false }, - "tasks[task_key='main_task'].notebook_task.source": { + "tasks[='main_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='main_task'].run_if": { + "tasks[='main_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main_task'].timeout_seconds": { + "tasks[='main_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json b/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json index d4a8e9cdc24..8ee97ff0b03 100644 --- a/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json +++ b/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json @@ -4,17 +4,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/jobs/whole-block-overlap/out.plan.direct.json b/acceptance/bundle/resources/jobs/whole-block-overlap/out.plan.direct.json index 03edfb9f1d8..e52a5a5c6a6 100644 --- a/acceptance/bundle/resources/jobs/whole-block-overlap/out.plan.direct.json +++ b/acceptance/bundle/resources/jobs/whole-block-overlap/out.plan.direct.json @@ -90,22 +90,22 @@ "old": "UTC", "new": "America/New_York" }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].notebook_task.source": { + "tasks[='main'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/databricks.yml.tmpl b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/databricks.yml.tmpl new file mode 100644 index 00000000000..20915edf952 --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/databricks.yml.tmpl @@ -0,0 +1,18 @@ +bundle: + name: dashboard-sp-perm-$UNIQUE_NAME + +resources: + dashboards: + foo: + display_name: test-dashboard-$UNIQUE_NAME + warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID + serialized_dashboard: '{"pages":[{"name":"page1","displayName":"Page 1"}]}' + permissions: + # A service principal declared under user_name using its application-ID UUID. + # The Permissions API resolves the UUID to a service principal and returns it + # as service_principal_name on GET, so desired (user_name) and remote + # (service_principal_name) name the same principal under different fields. The + # plan matches the entry by value and treats that field difference as no + # change, so the deploy converges instead of reporting a perpetual update. + - level: CAN_MANAGE + user_name: aaaaaaaa-bbbb-4ccc-dddd-eeeeeeeeeeee diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/out.test.toml b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/out.test.toml new file mode 100644 index 00000000000..59b56a2037c --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/out.test.toml @@ -0,0 +1,3 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/output.txt b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/output.txt new file mode 100644 index 00000000000..5b16126ae72 --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/output.txt @@ -0,0 +1,35 @@ + +=== Deploy the bundle +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dashboard-sp-perm-[UNIQUE_NAME]/default/files... +Created dashboards.foo +Created dashboards.foo.permissions +Files: 4 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== Re-plan after deploy (expected: no changes) +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged + +=== Permissions node of the JSON plan{ + "action": "skip", + "changes": null +} + +=== Lower the permission level: real drift is still detected +>>> [CLI] bundle plan +update dashboards.foo.permissions + +Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged + +=== Permissions node of the JSON plan{ + "action": "update", + "changes": { + "[='[UUID]'].level": { + "action": "update", + "old": "CAN_MANAGE", + "new": "CAN_READ", + "remote": "CAN_MANAGE" + } + } +} diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/script b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/script new file mode 100644 index 00000000000..fec69dab14e --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/script @@ -0,0 +1,23 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +title "Deploy the bundle" +trace $CLI bundle deploy +replace_ids.py + +# The service principal is declared under user_name but the API returns it as +# service_principal_name. The entry is matched by value and the field difference is +# relaxed, so a re-plan reports no changes rather than a perpetual no-op "update". +title "Re-plan after deploy (expected: no changes)" +trace $CLI bundle plan + +title "Permissions node of the JSON plan" +$CLI bundle plan -o json | jq '.plan["resources.dashboards.foo.permissions"] | {action, changes}' + +# A genuine change to the same principal must still be detected: lowering the level +# leaves the principal-field difference relaxed but reports the level update. +title "Lower the permission level: real drift is still detected" +update_file.py databricks.yml "CAN_MANAGE" "CAN_READ" +trace $CLI bundle plan + +title "Permissions node of the JSON plan" +$CLI bundle plan -o json | jq '.plan["resources.dashboards.foo.permissions"] | {action, changes}' diff --git a/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/test.toml b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/test.toml new file mode 100644 index 00000000000..7c25f8ee2f3 --- /dev/null +++ b/acceptance/bundle/resources/permissions/dashboards/sp_declared_as_user/test.toml @@ -0,0 +1,6 @@ +# Reproduction of a false-positive permissions "update" after migrating to the +# direct engine. Direct-only: the customer hit this on the direct engine and the +# JSON plan format is direct-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [".databricks", "databricks.yml"] diff --git a/acceptance/bundle/resources/permissions/jobs/added_remotely/out.plan.direct.json b/acceptance/bundle/resources/permissions/jobs/added_remotely/out.plan.direct.json index 30ef353f822..0c7527f9f0b 100644 --- a/acceptance/bundle/resources/permissions/jobs/added_remotely/out.plan.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/added_remotely/out.plan.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 @@ -112,7 +112,7 @@ ] }, "changes": { - "[group_name='admin-team']": { + "[='admin-team']": { "action": "update", "remote": { "level": "CAN_MANAGE", diff --git a/acceptance/bundle/resources/permissions/jobs/deleted_remotely/out.plan_restore.direct.json b/acceptance/bundle/resources/permissions/jobs/deleted_remotely/out.plan_restore.direct.json index d3172a577fe..b80d8217169 100644 --- a/acceptance/bundle/resources/permissions/jobs/deleted_remotely/out.plan_restore.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/deleted_remotely/out.plan_restore.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 @@ -108,7 +108,7 @@ ] }, "changes": { - "[group_name='data-team']": { + "[='data-team']": { "action": "update", "old": { "level": "CAN_MANAGE", @@ -119,7 +119,7 @@ "group_name": "data-team" } }, - "[user_name='viewer@example.com']": { + "[='viewer@example.com']": { "action": "update", "old": { "level": "CAN_VIEW", diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json index b6cfa725c53..3da1c793261 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_one.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_one.direct.json index 2f685883af6..95056616baf 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_one.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_one.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 @@ -112,7 +112,7 @@ ] }, "changes": { - "[group_name='data-team']": { + "[='data-team']": { "action": "update", "old": { "level": "CAN_MANAGE", diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_post_create.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_post_create.direct.json index 86c1144b0da..10a81c27a27 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_post_create.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_post_create.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_restore.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_restore.direct.json index e62b14f7ce6..95de154180e 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_restore.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_restore.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json index b01df3c6905..60838f6fc35 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_update.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_update.direct.json index 4c63a45fcd7..5221a23bd0e 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_update.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_update.direct.json @@ -44,17 +44,17 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='main'].email_notifications": { + "tasks[='main'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='main'].run_if": { + "tasks[='main'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='main'].timeout_seconds": { + "tasks[='main'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 @@ -116,7 +116,7 @@ ] }, "changes": { - "[user_name='viewer@example.com'].level": { + "[='viewer@example.com'].level": { "action": "update", "old": "CAN_VIEW", "new": "CAN_MANAGE", diff --git a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_one.direct.json b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_one.direct.json index 0891f67f199..03cf1a557bc 100644 --- a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_one.direct.json +++ b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_one.direct.json @@ -72,7 +72,7 @@ ] }, "changes": { - "[group_name='data-team']": { + "[='data-team']": { "action": "update", "old": { "level": "CAN_MANAGE", diff --git a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_update.direct.json b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_update.direct.json index cb89434d24d..c9724bc1199 100644 --- a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_update.direct.json +++ b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_update.direct.json @@ -76,7 +76,7 @@ ] }, "changes": { - "[user_name='viewer@example.com'].level": { + "[='viewer@example.com'].level": { "action": "update", "old": "CAN_VIEW", "new": "CAN_MANAGE", diff --git a/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_dev.direct.json b/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_dev.direct.json index 486cbafdb62..44a28ebc33f 100644 --- a/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_dev.direct.json +++ b/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_dev.direct.json @@ -229,64 +229,64 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].libraries[0].whl": { + "tasks[='notebook_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl" }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].libraries[0].whl": { + "tasks[='python_wheel_task'].libraries[0].whl": { "action": "update", "old": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl", "new": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl", "remote": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl" }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_prod.direct.json b/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_prod.direct.json index 8a060a51bc4..b0bc790b1c6 100644 --- a/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_prod.direct.json +++ b/acceptance/bundle/templates/default-python/classic/out.plan_after_deploy_prod.direct.json @@ -128,52 +128,52 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_dev.direct.json b/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_dev.direct.json index e50081e941d..91f4c50b3c3 100644 --- a/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_dev.direct.json +++ b/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_dev.direct.json @@ -117,52 +117,52 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_prod.direct.json b/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_prod.direct.json index 0746846796a..449f615398b 100644 --- a/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_prod.direct.json +++ b/acceptance/bundle/templates/default-python/serverless/out.plan_after_deploy_prod.direct.json @@ -114,52 +114,52 @@ "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].email_notifications": { + "tasks[='notebook_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='notebook_task'].notebook_task.source": { + "tasks[='notebook_task'].notebook_task.source": { "action": "skip", "reason": "backend_default", "remote": "WORKSPACE" }, - "tasks[task_key='notebook_task'].run_if": { + "tasks[='notebook_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='notebook_task'].timeout_seconds": { + "tasks[='notebook_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='python_wheel_task'].email_notifications": { + "tasks[='python_wheel_task'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='python_wheel_task'].run_if": { + "tasks[='python_wheel_task'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='python_wheel_task'].timeout_seconds": { + "tasks[='python_wheel_task'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 }, - "tasks[task_key='refresh_pipeline'].email_notifications": { + "tasks[='refresh_pipeline'].email_notifications": { "action": "skip", "reason": "empty", "remote": {} }, - "tasks[task_key='refresh_pipeline'].run_if": { + "tasks[='refresh_pipeline'].run_if": { "action": "skip", "reason": "backend_default", "remote": "ALL_SUCCESS" }, - "tasks[task_key='refresh_pipeline'].timeout_seconds": { + "tasks[='refresh_pipeline'].timeout_seconds": { "action": "skip", "reason": "empty", "remote": 0 diff --git a/bundle/configsync/keyfields.go b/bundle/configsync/keyfields.go new file mode 100644 index 00000000000..4e7d52b848a --- /dev/null +++ b/bundle/configsync/keyfields.go @@ -0,0 +1,76 @@ +package configsync + +import ( + "reflect" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/structs/registry" + "github.com/databricks/cli/libs/structs/structaccess" + "github.com/databricks/cli/libs/structs/structpath" +) + +// A keyed-slice element is addressed in a change path as [='value']: the key field is +// omitted because resolution is by value. configsync works on dynamic values, so it +// recovers the element's key fields from the Go type of the sequence — resolved +// against the bundle config schema — rather than guessing from field names (a task, +// for example, carries both task_key and a job_cluster_key reference, so a name-based +// guess would be ambiguous). + +// keyFieldsAtPath returns the key field JSON names of the element type of the sequence +// at seqPath (relative to the bundle config root), or nil if seqPath is not a keyed +// slice. +func keyFieldsAtPath(b *bundle.Bundle, seqPath *structpath.PathNode) []string { + seqType, err := structaccess.TypeAtPath(reflect.TypeOf(b.Config), seqPath) + if err != nil { + return nil + } + for seqType.Kind() == reflect.Pointer { + seqType = seqType.Elem() + } + if seqType.Kind() != reflect.Slice && seqType.Kind() != reflect.Array { + return nil + } + return registry.KeyFields(seqType.Elem()) +} + +// dynElementKey returns the identity of a dynamic keyed-slice element: the value of its +// first non-empty key field (keyFields resolved from the type), or "" when none is set. +// ok is false only when the element is not a mapping; a mapping with no key field set +// is addressable by the empty key, mirroring structaccess.ElementKeyValue. +func dynElementKey(elem dyn.Value, keyFields []string) (value string, ok bool) { + if _, isMap := elem.AsMap(); !isMap { + return "", false + } + for _, field := range keyFields { + v, err := dyn.GetByPath(elem, dyn.Path{dyn.Key(field)}) + if err != nil || v.Kind() != dyn.KindString { + continue + } + if s := v.MustString(); s != "" { + return s, true + } + } + // A mapping with no key field set is addressable by the empty key, mirroring + // structaccess.ElementKeyValue. + return "", true +} + +// singleKeyFieldAt returns the sole key field of the element addressed by keyedNode +// (a [='value'] selector, relative to resourceKey), or "" if the element type has no +// key field or more than one. Rename handling uses it to name the key it must strip +// and rewrite; a lone key field (task_key, name, …) is what rename applies to. +func singleKeyFieldAt(b *bundle.Bundle, resourceKey string, keyedNode *structpath.PathNode) string { + seqPath := keyedNode.Parent() + if seqPath == nil { + return "" + } + fullSeq, err := structpath.ParsePath(resourceKey + "." + seqPath.String()) + if err != nil { + return "" + } + if kf := keyFieldsAtPath(b, fullSeq); len(kf) == 1 { + return kf[0] + } + return "" +} diff --git a/bundle/configsync/rename.go b/bundle/configsync/rename.go index 916f37ef0c3..4a776275730 100644 --- a/bundle/configsync/rename.go +++ b/bundle/configsync/rename.go @@ -50,7 +50,7 @@ func matchRenamingPairs(b *bundle.Bundle, blocks *blockResolver, resourceKey str return set } - removes, adds := keyedElementChanges(changes) + removes, adds := keyedElementChanges(b, resourceKey, changes) // The remove half carries no value, so each removed element is read from the // merged configuration once: its fields decide which additions it could have @@ -239,7 +239,7 @@ type keyedElement struct { // 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) { +func keyedElementChanges(b *bundle.Bundle, resourceKey string, changes ResourceChanges) (removes, adds []keyedElement) { for _, path := range slices.Sorted(maps.Keys(changes)) { change := changes[path] if change.Operation != OperationRemove && change.Operation != OperationAdd { @@ -249,10 +249,20 @@ func keyedElementChanges(changes ResourceChanges) (removes, adds []keyedElement) if err != nil { continue } - keyField, key, ok := node.KeyValue() + _, key, ok := node.KeyValue() if !ok { continue } + // The path addresses the element by value only ([='value']), so the key field + // name is not in the path; recover it from the sequence's type. Renaming needs + // the name to strip the key when comparing bodies and to address the key field in + // the rewrite. Rename detection only applies to single-key-field elements (tasks, + // parameters, …); multi-key-field elements (permissions) never rename — a changed + // principal is an add plus a remove, not a rename — so they are skipped here. + keyField := singleKeyFieldAt(b, resourceKey, node) + if keyField == "" { + continue + } element := keyedElement{ path: path, parent: node.Parent().String(), diff --git a/bundle/configsync/resolve.go b/bundle/configsync/resolve.go index cdad5182336..d82d405fde7 100644 --- a/bundle/configsync/resolve.go +++ b/bundle/configsync/resolve.go @@ -105,13 +105,31 @@ func resolveSelectors(pathStr string, b *bundle.Bundle, operation OperationType) seq, _ := currentValue.AsSequence() foundIndex := -1 + // key is empty for the field-agnostic form [='value']: resolve the element by + // its registered key fields, looked up from the sequence's Go type. A named key + // (legacy form) matches that field directly. + var keyFields []string + if key == "" { + keyFields = keyFieldsAtPath(b, n.Parent()) + } + for i, elem := range seq { - keyValue, err := dyn.GetByPath(elem, dyn.Path{dyn.Key(key)}) - if err != nil { - continue + var elemKey string + if key == "" { + ek, ok := dynElementKey(elem, keyFields) + if !ok { + continue + } + elemKey = ek + } else { + keyValue, err := dyn.GetByPath(elem, dyn.Path{dyn.Key(key)}) + if err != nil || keyValue.Kind() != dyn.KindString { + continue + } + elemKey = keyValue.MustString() } - if keyValue.Kind() == dyn.KindString && keyValue.MustString() == value { + if elemKey == value { foundIndex = i break } diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 44b7061b6d6..5fb43893ef3 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -308,7 +308,7 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks logdiag.LogError(ctx, fmt.Errorf("%s: compacting local state: %w", errorPrefix, err)) return false } - localDiff, err := structdiff.GetStructDiff(compactedSavedState, localState, adapter.KeyedSlices()) + localDiff, err := structdiff.GetStructDiff(compactedSavedState, localState) if err != nil { logdiag.LogError(ctx, fmt.Errorf("%s: diffing local state: %w", errorPrefix, err)) return false @@ -349,7 +349,7 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks return false } - remoteDiff, err = structdiff.GetStructDiff(remoteStateComparable, localState, adapter.KeyedSlices()) + remoteDiff, err = structdiff.GetStructDiff(remoteStateComparable, localState) if err != nil { logdiag.LogError(ctx, fmt.Errorf("%s: diffing remote state: %w", errorPrefix, err)) return false diff --git a/bundle/direct/bundle_plan_test.go b/bundle/direct/bundle_plan_test.go index ccd7f827ad5..782832b6afb 100644 --- a/bundle/direct/bundle_plan_test.go +++ b/bundle/direct/bundle_plan_test.go @@ -563,9 +563,9 @@ func TestPrepareChangesWholeBlockOverlap(t *testing.T) { newState := threeWayOuter{Field: &threeWayMid{A: &threeWayInner{B: "old", C: "newc"}}} remote := threeWayOuter{Field: &threeWayMid{A: nil}} - localDiff, err := structdiff.GetStructDiff(old, newState, nil) + localDiff, err := structdiff.GetStructDiff(old, newState) require.NoError(t, err) - remoteDiff, err := structdiff.GetStructDiff(remote, newState, nil) + remoteDiff, err := structdiff.GetStructDiff(remote, newState) require.NoError(t, err) changes, err := prepareChanges(t.Context(), nil, localDiff, remoteDiff, old, remote) diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index 6210a952112..3065d335a16 100644 --- a/bundle/direct/dresources/adapter.go +++ b/bundle/direct/dresources/adapter.go @@ -108,10 +108,6 @@ type IResource interface { // timeout here leaves the bundle consistent (resource was requested deleted, retry on next plan). WaitAfterDelete(ctx context.Context, id string) error - // [Optional] KeyedSlices returns a map from path patterns to KeyFunc for comparing slices by key instead of by index. - // Example: func (*ResourcePermissions) KeyedSlices(state *PermissionsState) map[string]any - KeyedSlices() map[string]any - // [Optional] IsGone reports whether a remote resource should be treated as // already-deleted when planning a delete. Use for backends whose DELETE is // asynchronous and leaves the resource in a transient terminal-teardown state @@ -144,7 +140,6 @@ type Adapter struct { resourceConfig *ResourceLifecycleConfig generatedResourceConfig *ResourceLifecycleConfig - keyedSlices map[string]any } func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceClient) (*Adapter, error) { @@ -184,7 +179,6 @@ func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceC isGone: nil, resourceConfig: GetResourceConfig(resourceType), generatedResourceConfig: GetGeneratedResourceConfig(resourceType), - keyedSlices: nil, } err = adapter.initMethods(impl) @@ -213,16 +207,6 @@ func configureImpl(impl any, resourceType string) error { return err } -// loadKeyedSlices validates and calls KeyedSlices method, returning the resulting map. -func loadKeyedSlices(call *calladapt.BoundCaller) (map[string]any, error) { - outs, err := call.Call() - if err != nil { - return nil, fmt.Errorf("failed to call KeyedSlices: %w", err) - } - result := outs[0].(map[string]any) - return result, nil -} - func (a *Adapter) initMethods(resource any) error { err := calladapt.EnsureNoExtraMethods(resource, reflect.TypeFor[IResource]()) if err != nil { @@ -308,17 +292,6 @@ func (a *Adapter) initMethods(resource any) error { return err } - keyedSlicesCall, err := calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "KeyedSlices") - if err != nil { - return err - } - if keyedSlicesCall != nil { - a.keyedSlices, err = loadKeyedSlices(keyedSlicesCall) - if err != nil { - return err - } - } - return nil } @@ -694,12 +667,6 @@ func (a *Adapter) OverrideChangeDesc(ctx context.Context, path *structpath.PathN return err } -// KeyedSlices returns a map from path patterns to KeyFunc for comparing slices by key. -// If the resource doesn't implement KeyedSlices, returns nil. -func (a *Adapter) KeyedSlices() map[string]any { - return a.keyedSlices -} - // IsGone reports whether the remote state represents an already-deleted resource // for planning purposes. Resources that don't implement IsGone are never gone. func (a *Adapter) IsGone(remoteState any) bool { diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 96bc471399f..568229c3b29 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1068,7 +1068,7 @@ func (f *testIgnoreFilter) filterChanges(changes []structdiff.Change) []structdi // requireEqual compares two structs and fails the test if there are differences // that are not in the ignore_remote_changes list. func (f *testIgnoreFilter) requireEqual(t *testing.T, expected, actual any, msgAndArgs ...any) { - changes, err := structdiff.GetStructDiff(expected, actual, nil) + changes, err := structdiff.GetStructDiff(expected, actual) require.NoError(t, err) relevantChanges := f.filterChanges(changes) require.Empty(t, relevantChanges, msgAndArgs...) diff --git a/bundle/direct/dresources/grants.go b/bundle/direct/dresources/grants.go index 3aa48c58279..2167974ec54 100644 --- a/bundle/direct/dresources/grants.go +++ b/bundle/direct/dresources/grants.go @@ -7,11 +7,16 @@ import ( "slices" "strings" + "github.com/databricks/cli/libs/structs/registry" "github.com/databricks/cli/libs/structs/structvar" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) +func init() { + registry.Register[catalog.PrivilegeAssignment]("principal") +} + var grantResourceToSecurableType = map[string]string{ "catalogs": "catalog", "schemas": "schema", @@ -89,18 +94,6 @@ func (*ResourceGrants) IsEmptyState(state *GrantsState) bool { return len(state.EmbeddedSlice) == 0 } -func grantKey(x catalog.PrivilegeAssignment) (string, string) { - return "principal", x.Principal -} - -func (*ResourceGrants) KeyedSlices() map[string]any { - // Empty key because EmbeddedSlice appears at the root path of - // GrantsState (no "grants" prefix in struct walker paths). - return map[string]any{ - "": grantKey, - } -} - func (r *ResourceGrants) DoRead(ctx context.Context, id string) (*GrantsState, error) { securableType, fullName, err := parseGrantsID(id) if err != nil { diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index ba2cd8848dd..4f82a0c6761 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -6,12 +6,24 @@ import ( "strconv" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structs/registry" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/jobs" ) +func init() { + registry.Register[jobs.Task]("task_key") + registry.Register[jobs.JobParameterDefinition]("name") + registry.Register[jobs.JobCluster]("job_cluster_key") + registry.Register[jobs.JobEnvironment]("environment_key") + registry.Register[jobs.TaskDependency]("task_key") + // The Jobs API returns webhook_notifications.on_* in an arbitrary order, so keying + // them by id (at the job, task, and for_each task levels) avoids a phantom diff. + registry.Register[jobs.Webhook]("id") +} + // JobRemote is the return type for DoRead. It embeds JobSettings so that all // paths in StateType are valid paths in RemoteType. type JobRemote struct { @@ -56,64 +68,6 @@ func (*ResourceJob) RemapState(remote *JobRemote) *jobs.JobSettings { return &remote.JobSettings } -func getTaskKey(x jobs.Task) (string, string) { - return "task_key", x.TaskKey -} - -func getParameterName(x jobs.JobParameterDefinition) (string, string) { - return "name", x.Name -} - -func getJobClusterKey(x jobs.JobCluster) (string, string) { - return "job_cluster_key", x.JobClusterKey -} - -func getEnvironmentKey(x jobs.JobEnvironment) (string, string) { - return "environment_key", x.EnvironmentKey -} - -func getDependsOnTaskKey(x jobs.TaskDependency) (string, string) { - return "task_key", x.TaskKey -} - -func getWebhookKey(x jobs.Webhook) (string, string) { - return "id", x.Id -} - -// The Jobs API returns webhook_notifications.on_* in an arbitrary order, so -// diff them by id to avoid a phantom diff that never converges. -var webhookNotificationEvents = []string{ - "on_start", - "on_success", - "on_failure", - "on_duration_warning_threshold_exceeded", - "on_streaming_backlog_exceeded", -} - -// webhook_notifications appears at the job, task, and for_each task levels. -var webhookNotificationParents = []string{ - "webhook_notifications", - "tasks[*].webhook_notifications", - "tasks[*].for_each_task.task.webhook_notifications", -} - -func (*ResourceJob) KeyedSlices() map[string]any { - result := map[string]any{ - "tasks": getTaskKey, - "parameters": getParameterName, - "job_clusters": getJobClusterKey, - "environments": getEnvironmentKey, - "tasks[*].depends_on": getDependsOnTaskKey, - "tasks[*].for_each_task.task.depends_on": getDependsOnTaskKey, - } - for _, parent := range webhookNotificationParents { - for _, event := range webhookNotificationEvents { - result[parent+"."+event] = getWebhookKey - } - } - return result -} - func (r *ResourceJob) DoRead(ctx context.Context, id string) (*JobRemote, error) { idInt, err := parseJobID(id) if err != nil { diff --git a/bundle/direct/dresources/job_test.go b/bundle/direct/dresources/job_test.go index ca19620023f..59683218c11 100644 --- a/bundle/direct/dresources/job_test.go +++ b/bundle/direct/dresources/job_test.go @@ -32,8 +32,6 @@ func webhooks(ids ...string) []jobs.Webhook { // TestJobWebhookNotificationsOrderInsensitive verifies that a webhook list // reordered by the Jobs API produces no diff, but a changed set still does. func TestJobWebhookNotificationsOrderInsensitive(t *testing.T) { - keys := (&ResourceJob{}).KeyedSlices() - config := jobs.JobSettings{ WebhookNotifications: &jobs.WebhookNotifications{ OnSuccess: webhooks("a", "b", "c"), @@ -78,13 +76,13 @@ func TestJobWebhookNotificationsOrderInsensitive(t *testing.T) { }, } - changes, err := structdiff.GetStructDiff(config, remote, keys) + changes, err := structdiff.GetStructDiff(config, remote) require.NoError(t, err) assert.Empty(t, changes) // A genuinely different destination set is still detected. remote.WebhookNotifications.OnSuccess = webhooks("a", "b", "d") - changes, err = structdiff.GetStructDiff(config, remote, keys) + changes, err = structdiff.GetStructDiff(config, remote) require.NoError(t, err) assert.NotEmpty(t, changes) } diff --git a/bundle/direct/dresources/permissions.go b/bundle/direct/dresources/permissions.go index c2aebb4d0ac..f81b22500a5 100644 --- a/bundle/direct/dresources/permissions.go +++ b/bundle/direct/dresources/permissions.go @@ -6,11 +6,20 @@ import ( "reflect" "strings" + "github.com/databricks/cli/libs/structs/registry" "github.com/databricks/cli/libs/structs/structvar" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/iam" ) +func init() { + // A permission is identified by whichever principal field is set. The backend may + // return the same principal under a different field (e.g. a user_name holding a + // service principal's application ID comes back as service_principal_name), so all + // three are key fields and the field difference is not treated as a change. + registry.Register[StatePermission]("user_name", "service_principal_name", "group_name") +} + // GetAPIRequestObjectType is used by direct to construct a request to permissions API: // https://github.com/databricks/terraform-provider-databricks/blob/430902d/permissions/permission_definitions.go#L775C24-L775C32 var permissionResourceToObjectType = map[string]string{ @@ -146,27 +155,6 @@ func toStatePermissions(ps any) ([]StatePermission, error) { return result, nil } -func permissionKey(x StatePermission) (string, string) { - if x.UserName != "" { - return "user_name", x.UserName - } - if x.ServicePrincipalName != "" { - return "service_principal_name", x.ServicePrincipalName - } - if x.GroupName != "" { - return "group_name", x.GroupName - } - return "", "" -} - -func (*ResourcePermissions) KeyedSlices() map[string]any { - // Empty key because EmbeddedSlice appears at the root path of - // PermissionsState (no "permissions" prefix in struct walker paths). - return map[string]any{ - "": permissionKey, - } -} - // parsePermissionsID extracts the object type and ID from a permissions ID string. // Handles both 3-part IDs ("/jobs/123") and 4-part IDs ("/sql/warehouses/uuid"). func parsePermissionsID(id string) (extractedType, extractedID string, err error) { diff --git a/bundle/direct/dresources/secret_scope_acls.go b/bundle/direct/dresources/secret_scope_acls.go index 3e61a9d7801..c9e3f0382b2 100644 --- a/bundle/direct/dresources/secret_scope_acls.go +++ b/bundle/direct/dresources/secret_scope_acls.go @@ -7,12 +7,17 @@ import ( "strings" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structs/registry" "github.com/databricks/cli/libs/structs/structvar" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/workspace" ) +func init() { + registry.Register[workspace.AclItem]("principal") +} + type ResourceSecretScopeAcls struct { client *databricks.WorkspaceClient } @@ -63,16 +68,6 @@ func (*ResourceSecretScopeAcls) PrepareInputConfig(inputConfig *[]resources.Secr }, nil } -func aclItemKey(x workspace.AclItem) (string, string) { - return "principal", x.Principal -} - -func (*ResourceSecretScopeAcls) KeyedSlices() map[string]any { - return map[string]any{ - "acls": aclItemKey, - } -} - func (r *ResourceSecretScopeAcls) DoRead(ctx context.Context, id string) (*SecretScopeAclsState, error) { // id is the scope name currentAcls, err := r.client.Secrets.ListAclsAll(ctx, workspace.ListAclsRequest{ diff --git a/bundle/direct/dresources/serialize_test.go b/bundle/direct/dresources/serialize_test.go index 17b943a4e3e..bbaaca7d4a0 100644 --- a/bundle/direct/dresources/serialize_test.go +++ b/bundle/direct/dresources/serialize_test.go @@ -54,7 +54,7 @@ func assertJSONRoundTrip(t *testing.T, v any, label string) { // ForceSendFields and json:"-" fields, which are intentionally not serialized. // Free-form any fields must be populated with []any/map[string]any (as JSON // decoding yields) so they round-trip to the same concrete type. - changes, err := structdiff.GetStructDiff(v, back, nil) + changes, err := structdiff.GetStructDiff(v, back) require.NoError(t, err) require.Empty(t, changes, "%s lost %d field(s) in JSON round-trip:%s", label, len(changes), formatChanges(changes)) } diff --git a/libs/structs/registry/registry.go b/libs/structs/registry/registry.go new file mode 100644 index 00000000000..487eeb986f1 --- /dev/null +++ b/libs/structs/registry/registry.go @@ -0,0 +1,50 @@ +// Package registry records, per Go type, which fields identify the elements of a +// keyed slice. +// +// Diffing and path resolution match keyed-slice elements by the value of these +// fields instead of by position, and address an element in a path as [='value']: +// the key field is omitted from the path because resolution is by value, using this +// registry. Several key fields express a one-of identity — a permission is a +// user_name OR a service_principal_name OR a group_name — and an element's identity +// is the value of its first non-empty key field. +// +// The design assumes an element's key value is unique across its key fields within a +// slice: a user, service principal, and group never share the same value string +// (emails, application IDs, and group names occupy disjoint value spaces), so a +// value alone unambiguously identifies the element. +// +// This package only records the key field names; reading a field's value from an +// element is left to structaccess, so field resolution matches encoding/json exactly +// and stays consistent with the rest of the codebase. +// +// Types register their key fields from init(), keeping the declaration next to the +// type it describes: +// +// func init() { registry.Register[jobs.Task]("task_key") } +package registry + +import "reflect" + +// registrations is populated only from init() (before any goroutine runs), then read +// concurrently, so it needs no lock. +var registrations = map[reflect.Type][]string{} + +// Register declares the key fields (JSON names) that identify elements of slice +// element type T, in priority order. Intended to be called from init(). Pointer types +// are normalized to their element type, so Register[*T] and Register[T] agree. +func Register[T any](keyFields ...string) { + registrations[deref(reflect.TypeFor[T]())] = keyFields +} + +// KeyFields returns the key field JSON names for element type t (pointers +// dereferenced), or nil if t is not a registered keyed-slice element. +func KeyFields(t reflect.Type) []string { + return registrations[deref(t)] +} + +func deref(t reflect.Type) reflect.Type { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + return t +} diff --git a/libs/structs/registry/registry_test.go b/libs/structs/registry/registry_test.go new file mode 100644 index 00000000000..579a8defe7e --- /dev/null +++ b/libs/structs/registry/registry_test.go @@ -0,0 +1,42 @@ +package registry + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +type single struct { + Key string `json:"key,omitempty"` +} + +type multi struct { + UserName string `json:"user_name,omitempty"` + SpName string `json:"service_principal_name,omitempty"` +} + +type ptrKeyed struct { + Name string `json:"pk_name,omitempty"` +} + +func init() { + Register[single]("key") + Register[multi]("user_name", "service_principal_name") + Register[*ptrKeyed]("pk_name") +} + +func TestKeyFields(t *testing.T) { + assert.Equal(t, []string{"key"}, KeyFields(reflect.TypeFor[single]())) + assert.Equal(t, []string{"user_name", "service_principal_name"}, KeyFields(reflect.TypeFor[multi]())) + // Unregistered types return nil. + assert.Nil(t, KeyFields(reflect.TypeFor[struct{ X string }]())) +} + +func TestKeyFieldsPointerNormalization(t *testing.T) { + // Register[*T] normalizes to T, and lookups by either T or *T agree. + assert.Equal(t, []string{"pk_name"}, KeyFields(reflect.TypeFor[ptrKeyed]())) + assert.Equal(t, []string{"pk_name"}, KeyFields(reflect.TypeFor[*ptrKeyed]())) + // A registered value type is also found through a pointer. + assert.Equal(t, []string{"key"}, KeyFields(reflect.TypeFor[*single]())) +} diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 9a10f45aa56..7682e964957 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -6,6 +6,7 @@ import ( "reflect" "slices" + "github.com/databricks/cli/libs/structs/registry" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/structs/structtag" ) @@ -204,6 +205,21 @@ func accessKeyValue(v reflect.Value, key, value string, path *structpath.PathNod return reflect.Value{}, fmt.Errorf("%s: key-value syntax requires slice elements to be structs, got %s", path.String(), elemDeref.Kind()) } + if key == "" { + // Field-agnostic key ([='value']): the key field is not encoded in the path, + // so identify the element from its type via the registry. Its key value is the + // same identity the diff used, so the same principal under a different field + // (e.g. user_name vs service_principal_name) resolves consistently. + keyFields := registry.KeyFields(elemDeref.Type()) + if keyFields == nil { + return reflect.Value{}, fmt.Errorf("%s: field-agnostic key on unregistered element type %s", path.String(), elemDeref.Type()) + } + if elemKey, ok := ElementKeyValue(elemDeref, keyFields); ok && elemKey == value { + return elem, nil + } + continue + } + // Try to get the field value fieldVal, err := accessKey(elemDeref, key, path) if err != nil { @@ -228,6 +244,24 @@ func accessKeyValue(v reflect.Value, key, value string, path *structpath.PathNod return reflect.Value{}, &NotFoundError{fmt.Sprintf("%s: no element found with %s=%q", path.String(), key, value)} } +// ElementKeyValue returns the identity of a keyed-slice element: the value of its +// first non-empty key field (keyFields in priority order), or "" when no key field is +// set. Fields resolve the same way as elsewhere in structaccess, so the result matches +// encoding/json. ok is false only if elem is a nil pointer or not a struct; an element +// with no key set is still addressable, by the empty key (""). +func ElementKeyValue(elem reflect.Value, keyFields []string) (string, bool) { + elem, ok := deref(elem) + if !ok || elem.Kind() != reflect.Struct { + return "", false + } + for _, field := range keyFields { + if fv, _, _, found := findStructFieldByKey(elem, field); found && fv.Kind() == reflect.String && fv.String() != "" { + return fv.String(), true + } + } + return "", true +} + // findFieldInStruct searches for a field by JSON key in a single struct (no embedding). // Returns: fieldValue, structField, found func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.StructField, bool) { diff --git a/libs/structs/structaccess/get_test.go b/libs/structs/structaccess/get_test.go index 7ae556c8ae6..9999a3c79b3 100644 --- a/libs/structs/structaccess/get_test.go +++ b/libs/structs/structaccess/get_test.go @@ -862,3 +862,35 @@ func TestGetKeyValue_NestedMultiple(t *testing.T) { require.NoError(t, err) require.Equal(t, "item3", name) } + +type keElem struct { + UserName string `json:"user_name,omitempty"` + SpName string `json:"service_principal_name,omitempty"` + Level string `json:"level,omitempty"` +} + +func TestElementKeyValue(t *testing.T) { + keys := []string{"user_name", "service_principal_name"} + + // First non-empty key field wins. + v, ok := ElementKeyValue(reflect.ValueOf(keElem{SpName: "sp"}), keys) + require.True(t, ok) + require.Equal(t, "sp", v) + + v, ok = ElementKeyValue(reflect.ValueOf(keElem{UserName: "u", SpName: "sp"}), keys) + require.True(t, ok) + require.Equal(t, "u", v) + + // No key field set: addressable by the empty key (ok true, value ""). + v, ok = ElementKeyValue(reflect.ValueOf(keElem{Level: "CAN_MANAGE"}), keys) + require.True(t, ok) + require.Empty(t, v) + + // Pointer element is dereferenced; nil pointer is not ok. + v, ok = ElementKeyValue(reflect.ValueOf(&keElem{UserName: "u"}), keys) + require.True(t, ok) + require.Equal(t, "u", v) + + _, ok = ElementKeyValue(reflect.ValueOf((*keElem)(nil)), keys) + require.False(t, ok) +} diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index e736daf877c..1f704fa87b9 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -489,7 +489,7 @@ func TestSet(t *testing.T) { require.NoError(t, err) // Compare the actual changes using structdiff - changes, err := structdiff.GetStructDiff(original, target, nil) + changes, err := structdiff.GetStructDiff(original, target) require.NoError(t, err) assert.Equal(t, tt.expectedChanges, changes) }) diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index b55fa4c136a..e232c366e99 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -41,11 +41,22 @@ func ValidatePattern(t reflect.Type, path *structpath.PatternNode) error { if path.IsRoot() { return nil } - return validateNodeSlice(t, path.AsSlice()) + _, err := resolveNodeSlice(t, path.AsSlice()) + return err } -// validateNodeSlice is the implementation for ValidatePattern. -func validateNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) error { +// TypeAtPath returns the type reached by following path from t, dereferencing pointers. +// The root (nil path) resolves to t. It returns an error indicating where resolution +// failed, mirroring ValidatePath. +func TypeAtPath(t reflect.Type, path *structpath.PathNode) (reflect.Type, error) { + if path.IsRoot() { + return t, nil + } + return resolveNodeSlice(t, (*structpath.PatternNode)(path).AsSlice()) +} + +// resolveNodeSlice walks nodes from t and returns the type reached, or an error. +func resolveNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) (reflect.Type, error) { cur := t for _, node := range nodes { // Always dereference pointers at the type level. @@ -67,7 +78,7 @@ func validateNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) error { if idx == 0 && kind == reflect.Struct { continue } - return fmt.Errorf("%s: cannot index %s", node.String(), kind) + return nil, fmt.Errorf("%s: cannot index %s", node.String(), kind) } cur = cur.Elem() continue @@ -82,14 +93,14 @@ func validateNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) error { } kind := cur.Kind() if kind != reflect.Slice && kind != reflect.Array { - return fmt.Errorf("%s: cannot use [*] on %s", node.String(), kind) + return nil, fmt.Errorf("%s: cannot use [*] on %s", node.String(), kind) } cur = cur.Elem() continue } if node.DotStar() { if cur.Kind() != reflect.Map { - return fmt.Errorf("%s: cannot use .* on %s", node.String(), cur.Kind()) + return nil, fmt.Errorf("%s: cannot use .* on %s", node.String(), cur.Kind()) } cur = cur.Elem() continue @@ -104,7 +115,7 @@ func validateNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) error { } kind := cur.Kind() if kind != reflect.Slice && kind != reflect.Array { - return fmt.Errorf("%s: cannot use key-value syntax on %s", node.String(), kind) + return nil, fmt.Errorf("%s: cannot use key-value syntax on %s", node.String(), kind) } cur = cur.Elem() continue @@ -113,28 +124,28 @@ func validateNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) error { key, ok := node.StringKey() if !ok { - return errors.New("unsupported path node type") + return nil, errors.New("unsupported path node type") } switch cur.Kind() { case reflect.Struct: sf, _, ok := FindStructFieldByKeyType(cur, key) if !ok { - return fmt.Errorf("%s: field %q not found in %s", node.String(), key, cur.String()) + return nil, fmt.Errorf("%s: field %q not found in %s", node.String(), key, cur.String()) } cur = sf.Type case reflect.Map: kt := cur.Key() if kt.Kind() != reflect.String { - return fmt.Errorf("%s: map key must be string, got %s", node.String(), kt) + return nil, fmt.Errorf("%s: map key must be string, got %s", node.String(), kt) } cur = cur.Elem() default: - return fmt.Errorf("%s: cannot access key %q on %s", node.String(), key, cur.Kind()) + return nil, fmt.Errorf("%s: cannot access key %q on %s", node.String(), key, cur.Kind()) } } - return nil + return cur, nil } // FindStructFieldByKeyType searches exported fields of struct type t for a field matching key. diff --git a/libs/structs/structdiff/bench_test.go b/libs/structs/structdiff/bench_test.go index 08b386416ef..1421b075202 100644 --- a/libs/structs/structdiff/bench_test.go +++ b/libs/structs/structdiff/bench_test.go @@ -18,7 +18,7 @@ func bench(b *testing.B, job1, job2 string) { total := 0 for b.Loop() { - changes, err := GetStructDiff(&x, &y, nil) + changes, err := GetStructDiff(&x, &y) if err != nil { b.Fatalf("error: %s", err) } diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index 5e7a00cac4f..a221a37a479 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -5,8 +5,8 @@ import ( "maps" "reflect" "slices" - "strings" + "github.com/databricks/cli/libs/structs/registry" "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/structs/structtag" @@ -18,64 +18,17 @@ type Change struct { New any } -// KeyFunc extracts a key field name and value from a slice element. -// It can be either: -// - func(T) (string, string) - typed function for specific element type T -// - func(any) (string, string) - generic function accepting any element -// -// The function returns (keyField, keyValue). The keyField is typically a field name -// like "task_key", and keyValue is the value that uniquely identifies the element. -type KeyFunc = any - -// keyFuncCaller wraps a KeyFunc and provides a type-checked Call method. -type keyFuncCaller struct { - fn reflect.Value - argType reflect.Type -} - -func newKeyFuncCaller(fn any) (*keyFuncCaller, error) { - v := reflect.ValueOf(fn) - if v.Kind() != reflect.Func { - return nil, fmt.Errorf("KeyFunc must be a function, got %T", fn) - } - t := v.Type() - if t.NumIn() != 1 { - return nil, fmt.Errorf("KeyFunc must have exactly 1 parameter, got %d", t.NumIn()) - } - if t.NumOut() != 2 { - return nil, fmt.Errorf("KeyFunc must return exactly 2 values, got %d", t.NumOut()) - } - if t.Out(0).Kind() != reflect.String || t.Out(1).Kind() != reflect.String { - return nil, fmt.Errorf("KeyFunc must return (string, string), got (%v, %v)", t.Out(0), t.Out(1)) - } - return &keyFuncCaller{fn: v, argType: t.In(0)}, nil -} - -func (c *keyFuncCaller) call(elem any) (string, string) { - elemValue := reflect.ValueOf(elem) - out := c.fn.Call([]reflect.Value{elemValue}) - keyField := out[0].String() - keyValue := out[1].String() - return keyField, keyValue -} - -// diffContext holds configuration for the diff operation. -type diffContext struct { - sliceKeys map[string]KeyFunc -} - // GetStructDiff compares two Go structs and returns a list of Changes or an error. // Respects ForceSendFields if present. // Types of a and b must match exactly, otherwise returns an error. // -// The sliceKeys parameter maps path patterns to functions that extract -// key field/value pairs from slice elements. When provided, slices at matching -// paths are compared as maps keyed by (keyField, keyValue) instead of by index. -// Path patterns use dot notation (e.g., "tasks" or "job.tasks"). -// The [*] wildcard matches any slice index in the path. -// Note, key wildcard is not supported yet ("a.*.c") -// Pass nil if no slice key functions are needed. -func GetStructDiff(a, b any, sliceKeys map[string]KeyFunc) ([]Change, error) { +// A slice whose element type is registered as a keyed slice (see libs/structs/registry) +// is matched by key value instead of by index. The element is addressed in the path as +// [='value'] — the key field is omitted, because the value identifies the element. +// A diff on a matched element's own key field is therefore not a real change and is +// dropped, which lets the same identity carried under a different field (e.g. user_name +// vs service_principal_name for one principal) compare equal. +func GetStructDiff(a, b any) ([]Change, error) { v1 := reflect.ValueOf(a) v2 := reflect.ValueOf(b) @@ -94,8 +47,7 @@ func GetStructDiff(a, b any, sliceKeys map[string]KeyFunc) ([]Change, error) { return nil, fmt.Errorf("type mismatch: %v vs %v", v1.Type(), v2.Type()) } - ctx := &diffContext{sliceKeys: sliceKeys} - if err := diffValues(ctx, nil, v1, v2, &changes); err != nil { + if err := diffValues(nil, v1, v2, &changes); err != nil { return nil, err } return changes, nil @@ -103,7 +55,7 @@ func GetStructDiff(a, b any, sliceKeys map[string]KeyFunc) ([]Change, error) { // diffValues appends changes between v1 and v2 to the slice. path is the current // JSON-style path (dot + brackets). At the root path is "". -func diffValues(ctx *diffContext, path *structpath.PathNode, v1, v2 reflect.Value, changes *[]Change) error { +func diffValues(path *structpath.PathNode, v1, v2 reflect.Value, changes *[]Change) error { if !v1.IsValid() { if !v2.IsValid() { return nil @@ -164,25 +116,25 @@ func diffValues(ctx *diffContext, path *structpath.PathNode, v1, v2 reflect.Valu switch kind { case reflect.Pointer: - return diffValues(ctx, path, v1.Elem(), v2.Elem(), changes) + return diffValues(path, v1.Elem(), v2.Elem(), changes) case reflect.Struct: - return diffStruct(ctx, path, v1, v2, changes) + return diffStruct(path, v1, v2, changes) case reflect.Slice, reflect.Array: - if keyFunc := ctx.findKeyFunc(path); keyFunc != nil { - return diffSliceByKey(ctx, path, v1, v2, keyFunc, changes) + if keyFields := registry.KeyFields(v1Type.Elem()); keyFields != nil { + return diffSliceByKey(path, v1, v2, keyFields, changes) } else if v1.Len() != v2.Len() { *changes = append(*changes, Change{Path: path, Old: v1.Interface(), New: v2.Interface()}) } else { for i := range v1.Len() { node := structpath.NewIndex(path, i) - if err := diffValues(ctx, node, v1.Index(i), v2.Index(i), changes); err != nil { + if err := diffValues(node, v1.Index(i), v2.Index(i), changes); err != nil { return err } } } case reflect.Map: if v1Type.Key().Kind() == reflect.String { - return diffMapStringKey(ctx, path, v1, v2, changes) + return diffMapStringKey(path, v1, v2, changes) } else { deepEqualValues(path, v1, v2, changes) } @@ -198,7 +150,7 @@ func deepEqualValues(path *structpath.PathNode, v1, v2 reflect.Value, changes *[ } } -func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Value, changes *[]Change) error { +func diffStruct(path *structpath.PathNode, s1, s2 reflect.Value, changes *[]Change) error { t := s1.Type() forced1 := getForceSendFields(s1) forced2 := getForceSendFields(s2) @@ -213,7 +165,7 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu // anonymous field carrying a json name is not one of these: encoding/json serializes it // as a nested object, so it is handled as a named field below. if structaccess.IsFlattenedEmbed(sf) { - if err := diffValues(ctx, path, s1.Field(i), s2.Field(i), changes); err != nil { + if err := diffValues(path, s1.Field(i), s2.Field(i), changes); err != nil { return err } continue @@ -262,14 +214,14 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu node = structpath.NewDotString(path, fieldName) } - if err := diffValues(ctx, node, v1Field, v2Field, changes); err != nil { + if err := diffValues(node, v1Field, v2Field, changes); err != nil { return err } } return nil } -func diffMapStringKey(ctx *diffContext, path *structpath.PathNode, m1, m2 reflect.Value, changes *[]Change) error { +func diffMapStringKey(path *structpath.PathNode, m1, m2 reflect.Value, changes *[]Change) error { keySet := map[string]reflect.Value{} for _, k := range m1.MapKeys() { // Caller guarantees the key kind is String; use Value.String() rather @@ -288,7 +240,7 @@ func diffMapStringKey(ctx *diffContext, path *structpath.PathNode, m1, m2 reflec v1 := m1.MapIndex(k) v2 := m2.MapIndex(k) node := structpath.NewBracketString(path, ks) - if err := diffValues(ctx, node, v1, v2, changes); err != nil { + if err := diffValues(node, v1, v2, changes); err != nil { return err } } @@ -310,138 +262,74 @@ func getForceSendFields(v reflect.Value) []string { return nil } -// findKeyFunc returns the KeyFunc for the given path, or nil if none matches. -// Path patterns support [*] to match any slice index. -func (ctx *diffContext) findKeyFunc(path *structpath.PathNode) KeyFunc { - if ctx.sliceKeys == nil { - return nil - } - pathStr := pathToPattern(path) - return ctx.sliceKeys[pathStr] -} - -// pathToPattern converts a PathNode to a pattern string for matching. -// Slice indices and key-value pairs are converted to [*] wildcard. -func pathToPattern(path *structpath.PathNode) string { - if path == nil { - return "" - } - - components := path.AsSlice() - var result strings.Builder - - for i, node := range components { - if _, ok := node.Index(); ok { - result.WriteString("[*]") - } else if _, _, ok := node.KeyValue(); ok { - result.WriteString("[*]") - } else if key, ok := node.StringKey(); ok { - if i != 0 { - result.WriteString(".") +// appendSkippingKeyFields appends pairChanges to changes, dropping any change on an +// element's own key field: a direct child of node whose field name is one of +// keyFields. Such a diff is not a real change — the element is matched by key value, +// so a key-field difference only means the same identity is carried under a different +// field (e.g. service_principal_name vs user_name). +func appendSkippingKeyFields(changes *[]Change, pairChanges []Change, node *structpath.PathNode, keyFields []string) { + for _, ch := range pairChanges { + if ch.Path.Parent() == node { + if field, ok := ch.Path.StringKey(); ok && slices.Contains(keyFields, field) { + continue } - result.WriteString(key) } + *changes = append(*changes, ch) } - - return result.String() } -// sliceElement holds a slice element with its key information. -type sliceElement struct { - keyField string - keyValue string - value reflect.Value -} - -// validateKeyFuncElementType verifies that the first element type in the sequence -// is assignable to the expected type. If the sequence is empty, it succeeds. -func validateKeyFuncElementType(seq reflect.Value, expected reflect.Type) error { - if seq.Len() == 0 { - return nil - } - elem := seq.Index(0) - if !elem.Type().AssignableTo(expected) { - return fmt.Errorf("KeyFunc expects %v, got %v", expected, elem.Type()) - } - return nil -} - -// diffSliceByKey compares two slices using the provided key function. -// Elements are matched by their (keyField, keyValue) pairs instead of by index. -// Duplicate keys are allowed and matched in order. -func diffSliceByKey(ctx *diffContext, path *structpath.PathNode, v1, v2 reflect.Value, keyFunc KeyFunc, changes *[]Change) error { - caller, err := newKeyFuncCaller(keyFunc) - if err != nil { - return err - } - - // Validate element types up-front to avoid runtime panics and to return a clear error. - if err := validateKeyFuncElementType(v1, caller.argType); err != nil { - return err - } - if err := validateKeyFuncElementType(v2, caller.argType); err != nil { - return err - } - - // Build lists of elements grouped by key, preserving order within each key - elements1 := make(map[string][]sliceElement) - elements2 := make(map[string][]sliceElement) +// diffSliceByKey compares two slices whose element type is a registered keyed slice. +// Elements are matched by their key value (registry.ElementKey) instead of by index, +// and addressed as [='value']. Duplicate keys are allowed and matched in order. +func diffSliceByKey(path *structpath.PathNode, v1, v2 reflect.Value, keyFields []string, changes *[]Change) error { + // Build lists of elements grouped by key value, preserving order within each key. + elements1 := make(map[string][]reflect.Value) + elements2 := make(map[string][]reflect.Value) seen := make(map[string]bool) var orderedKeys []string - // Build from first slice - for i := range v1.Len() { - elem := v1.Index(i) - keyField, keyValue := caller.call(elem.Interface()) - elements1[keyValue] = append(elements1[keyValue], sliceElement{keyField: keyField, keyValue: keyValue, value: elem}) - if !seen[keyValue] { - seen[keyValue] = true - orderedKeys = append(orderedKeys, keyValue) - } - } - - // Build from second slice - for i := range v2.Len() { - elem := v2.Index(i) - keyField, keyValue := caller.call(elem.Interface()) - elements2[keyValue] = append(elements2[keyValue], sliceElement{keyField: keyField, keyValue: keyValue, value: elem}) - if !seen[keyValue] { - seen[keyValue] = true - orderedKeys = append(orderedKeys, keyValue) + group := func(v reflect.Value, into map[string][]reflect.Value) { + for i := range v.Len() { + elem := v.Index(i) + key, _ := structaccess.ElementKeyValue(elem, keyFields) + into[key] = append(into[key], elem) + if !seen[key] { + seen[key] = true + orderedKeys = append(orderedKeys, key) + } } } + group(v1, elements1) + group(v2, elements2) - // Compare elements by key in original order - for _, keyValue := range orderedKeys { - list1 := elements1[keyValue] - list2 := elements2[keyValue] + for _, key := range orderedKeys { + list1 := elements1[key] + list2 := elements2[key] - var keyField string - if len(list1) > 0 { - keyField = list1[0].keyField - } else { - keyField = list2[0].keyField - } + // The element is addressed by value only ([='value']); the key field is omitted, + // so both sides render identically even when they carry the same identity under a + // different field. Resolution back to an element is the type-aware resolver's job + // (structaccess, via the registry). + node := structpath.NewKeyValue(path, "", key) - // Match elements in order + // Match elements in order. minLen := min(len(list1), len(list2)) for i := range minLen { - node := structpath.NewKeyValue(path, keyField, keyValue) - if err := diffValues(ctx, node, list1[i].value, list2[i].value, changes); err != nil { + var pairChanges []Change + if err := diffValues(node, list1[i], list2[i], &pairChanges); err != nil { return err } + appendSkippingKeyFields(changes, pairChanges, node, keyFields) } // Handle extra elements in old (deleted) for i := minLen; i < len(list1); i++ { - node := structpath.NewKeyValue(path, keyField, keyValue) - *changes = append(*changes, Change{Path: node, Old: list1[i].value.Interface(), New: nil}) + *changes = append(*changes, Change{Path: node, Old: list1[i].Interface(), New: nil}) } // Handle extra elements in new (added) for i := minLen; i < len(list2); i++ { - node := structpath.NewKeyValue(path, keyField, keyValue) - *changes = append(*changes, Change{Path: node, Old: nil, New: list2[i].value.Interface()}) + *changes = append(*changes, Change{Path: node, Old: nil, New: list2[i].Interface()}) } } return nil diff --git a/libs/structs/structdiff/diff_test.go b/libs/structs/structdiff/diff_test.go index dd1cf4bb5ad..bfdad87741c 100644 --- a/libs/structs/structdiff/diff_test.go +++ b/libs/structs/structdiff/diff_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/databricks/cli/libs/structs/registry" "github.com/databricks/databricks-sdk-go/common/types/duration" sdktime "github.com/databricks/databricks-sdk-go/common/types/time" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -13,6 +14,17 @@ import ( "github.com/stretchr/testify/require" ) +// Register the local test types used by the keyed-slice tests below. EmbedItem is +// deliberately left unregistered so TestGetStructDiffEmbedTag exercises the unkeyed +// (index/whole-slice) path. +func init() { + registry.Register[Task]("task_key") + registry.Register[Dep]("task_key") + registry.Register[Item]("id") + registry.Register[principal]("user_name", "service_principal_name") + registry.Register[keyedEmbedItem]("name") +} + type B struct{ S string } type A struct { @@ -442,7 +454,7 @@ func TestGetStructDiff(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, nil) + got, err := GetStructDiff(tt.a, tt.b) assert.Equal(t, tt.want, resolveChanges(got)) @@ -454,7 +466,7 @@ func TestGetStructDiff(t *testing.T) { }) t.Run(tt.name+" mirror", func(t *testing.T) { - got, err := GetStructDiff(tt.b, tt.a, nil) + got, err := GetStructDiff(tt.b, tt.a) var mirrorWant []ResolvedChange for _, ch := range tt.want { @@ -475,13 +487,13 @@ func TestGetStructDiff(t *testing.T) { }) t.Run(tt.name+" equal A", func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.a, nil) + got, err := GetStructDiff(tt.a, tt.a) assert.NoError(t, err) assert.Nil(t, got) }) t.Run(tt.name+" equal B", func(t *testing.T) { - got, err := GetStructDiff(tt.b, tt.b, nil) + got, err := GetStructDiff(tt.b, tt.b) assert.NoError(t, err) assert.Nil(t, got) }) @@ -504,10 +516,6 @@ type EmbedContainer struct { EmbeddedSlice []EmbedItem `json:"items,omitempty"` } -func embedItemKey(item EmbedItem) (string, string) { - return "name", item.Name -} - func TestGetStructDiffEmbedTag(t *testing.T) { tests := []struct { name string @@ -549,53 +557,111 @@ func TestGetStructDiffEmbedTag(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, nil) + got, err := GetStructDiff(tt.a, tt.b) assert.NoError(t, err) assert.Equal(t, tt.want, resolveChanges(got)) }) } } -func TestGetStructDiffEmbedTagWithKeyFunc(t *testing.T) { - // The EmbeddedSlice field appears at root path, so key pattern is "". - sliceKeys := map[string]KeyFunc{ - "": embedItemKey, - } +// keyedEmbedItem is a registered keyed element (by "name"); keyedEmbedContainer holds +// it as an EmbeddedSlice, so the elements are addressed at the container's root path. +type keyedEmbedItem struct { + Name string `json:"name,omitempty"` + Level string `json:"level,omitempty"` +} + +type keyedEmbedContainer struct { + ObjectID string `json:"object_id"` + EmbeddedSlice []keyedEmbedItem `json:"items,omitempty"` +} +func TestGetStructDiffEmbedTagWithKeyFunc(t *testing.T) { tests := []struct { name string - a, b EmbedContainer + a, b keyedEmbedContainer want []ResolvedChange }{ { name: "reorder with key func", - a: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "alice", Level: "admin"}, {Name: "bob", Level: "reader"}}}, - b: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "bob", Level: "reader"}, {Name: "alice", Level: "admin"}}}, + a: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "alice", Level: "admin"}, {Name: "bob", Level: "reader"}}}, + b: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "bob", Level: "reader"}, {Name: "alice", Level: "admin"}}}, want: nil, }, { name: "field change with key func", - a: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "alice", Level: "admin"}}}, - b: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "alice", Level: "reader"}}}, - want: []ResolvedChange{{Field: "[name='alice'].level", Old: "admin", New: "reader"}}, + a: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "alice", Level: "admin"}}}, + b: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "alice", Level: "reader"}}}, + want: []ResolvedChange{{Field: "[='alice'].level", Old: "admin", New: "reader"}}, }, { name: "element added with key func", - a: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "alice"}}}, - b: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "alice"}, {Name: "bob", Level: "reader"}}}, - want: []ResolvedChange{{Field: "[name='bob']", Old: nil, New: EmbedItem{Name: "bob", Level: "reader"}}}, + a: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "alice"}}}, + b: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "alice"}, {Name: "bob", Level: "reader"}}}, + want: []ResolvedChange{{Field: "[='bob']", Old: nil, New: keyedEmbedItem{Name: "bob", Level: "reader"}}}, }, { name: "element removed with key func", - a: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "alice", Level: "admin"}, {Name: "bob"}}}, - b: EmbedContainer{ObjectID: "abc", EmbeddedSlice: []EmbedItem{{Name: "alice", Level: "admin"}}}, - want: []ResolvedChange{{Field: "[name='bob']", Old: EmbedItem{Name: "bob"}, New: nil}}, + a: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "alice", Level: "admin"}, {Name: "bob"}}}, + b: keyedEmbedContainer{ObjectID: "abc", EmbeddedSlice: []keyedEmbedItem{{Name: "alice", Level: "admin"}}}, + want: []ResolvedChange{{Field: "[='bob']", Old: keyedEmbedItem{Name: "bob"}, New: nil}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetStructDiff(tt.a, tt.b) + assert.NoError(t, err) + assert.Equal(t, tt.want, resolveChanges(got)) + }) + } +} + +// principal has two interchangeable identity fields (like a permission's +// user_name / service_principal_name) plus a non-key field. +type principal struct { + UserName string `json:"user_name,omitempty"` + SpName string `json:"service_principal_name,omitempty"` + Level string `json:"level,omitempty"` +} + +type principalContainer struct { + ObjectID string `json:"object_id"` + EmbeddedSlice []principal `json:"items,omitempty"` +} + +// TestGetStructDiffKeyFieldSwap covers a registered type whose key field varies per +// element (principal: user_name or service_principal_name): the same identity value +// carried under a different field is not a change, but a non-key field still diffs. +func TestGetStructDiffKeyFieldSwap(t *testing.T) { + tests := []struct { + name string + a, b principalContainer + want []ResolvedChange + }{ + { + name: "field swap only is not a change", + a: principalContainer{EmbeddedSlice: []principal{{SpName: "X", Level: "CAN_MANAGE"}}}, + b: principalContainer{EmbeddedSlice: []principal{{UserName: "X", Level: "CAN_MANAGE"}}}, + want: nil, + }, + { + name: "field swap with level change reports only the level", + a: principalContainer{EmbeddedSlice: []principal{{SpName: "X", Level: "CAN_MANAGE"}}}, + b: principalContainer{EmbeddedSlice: []principal{{UserName: "X", Level: "CAN_READ"}}}, + want: []ResolvedChange{{Field: "[='X'].level", Old: "CAN_MANAGE", New: "CAN_READ"}}, + }, + { + name: "same field, level change", + a: principalContainer{EmbeddedSlice: []principal{{UserName: "X", Level: "CAN_MANAGE"}}}, + b: principalContainer{EmbeddedSlice: []principal{{UserName: "X", Level: "CAN_READ"}}}, + want: []ResolvedChange{{Field: "[='X'].level", Old: "CAN_MANAGE", New: "CAN_READ"}}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, sliceKeys) + got, err := GetStructDiff(tt.a, tt.b) assert.NoError(t, err) assert.Equal(t, tt.want, resolveChanges(got)) }) @@ -619,19 +685,7 @@ type Job struct { Tasks []Task `json:"tasks,omitempty"` } -func taskKeyFunc(task Task) (string, string) { - return "task_key", task.TaskKey -} - -func depKeyFunc(dep Dep) (string, string) { - return "task_key", dep.TaskKey -} - func TestGetStructDiffSliceKeys(t *testing.T) { - sliceKeys := map[string]KeyFunc{ - "tasks": taskKeyFunc, - } - tests := []struct { name string a, b any @@ -653,27 +707,27 @@ func TestGetStructDiffSliceKeys(t *testing.T) { name: "slice with same keys field change", a: Job{Tasks: []Task{{TaskKey: "a", Description: "one"}}}, b: Job{Tasks: []Task{{TaskKey: "a", Description: "changed"}}}, - want: []ResolvedChange{{Field: "tasks[task_key='a'].description", Old: "one", New: "changed"}}, + want: []ResolvedChange{{Field: "tasks[='a'].description", Old: "one", New: "changed"}}, }, { name: "slice element added", a: Job{Tasks: []Task{{TaskKey: "a", Description: "one"}}}, b: Job{Tasks: []Task{{TaskKey: "a", Description: "one"}, {TaskKey: "b", Description: "two"}}}, - want: []ResolvedChange{{Field: "tasks[task_key='b']", Old: nil, New: Task{TaskKey: "b", Description: "two"}}}, + want: []ResolvedChange{{Field: "tasks[='b']", Old: nil, New: Task{TaskKey: "b", Description: "two"}}}, }, { name: "slice element removed", a: Job{Tasks: []Task{{TaskKey: "a", Description: "one"}, {TaskKey: "b", Description: "two"}}}, b: Job{Tasks: []Task{{TaskKey: "a", Description: "one"}}}, - want: []ResolvedChange{{Field: "tasks[task_key='b']", Old: Task{TaskKey: "b", Description: "two"}, New: nil}}, + want: []ResolvedChange{{Field: "tasks[='b']", Old: Task{TaskKey: "b", Description: "two"}, New: nil}}, }, { name: "slice element replaced", a: Job{Tasks: []Task{{TaskKey: "a", Description: "one"}}}, b: Job{Tasks: []Task{{TaskKey: "b", Description: "two"}}}, want: []ResolvedChange{ - {Field: "tasks[task_key='a']", Old: Task{TaskKey: "a", Description: "one"}, New: nil}, - {Field: "tasks[task_key='b']", Old: nil, New: Task{TaskKey: "b", Description: "two"}}, + {Field: "tasks[='a']", Old: Task{TaskKey: "a", Description: "one"}, New: nil}, + {Field: "tasks[='b']", Old: nil, New: Task{TaskKey: "b", Description: "two"}}, }, }, { @@ -681,15 +735,15 @@ func TestGetStructDiffSliceKeys(t *testing.T) { a: Job{Tasks: []Task{{TaskKey: "a", Description: "one"}, {TaskKey: "b", Description: "two"}, {TaskKey: "c", Description: "three"}}}, b: Job{Tasks: []Task{{TaskKey: "c", Description: "changed"}, {TaskKey: "a", Description: "one"}}}, want: []ResolvedChange{ - {Field: "tasks[task_key='b']", Old: Task{TaskKey: "b", Description: "two"}, New: nil}, - {Field: "tasks[task_key='c'].description", Old: "three", New: "changed"}, + {Field: "tasks[='b']", Old: Task{TaskKey: "b", Description: "two"}, New: nil}, + {Field: "tasks[='c'].description", Old: "three", New: "changed"}, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, sliceKeys) + got, err := GetStructDiff(tt.a, tt.b) assert.NoError(t, err) assert.Equal(t, tt.want, resolveChanges(got)) }) @@ -697,11 +751,6 @@ func TestGetStructDiffSliceKeys(t *testing.T) { } func TestGetStructDiffNestedDependsOn(t *testing.T) { - sliceKeys := map[string]KeyFunc{ - "tasks": taskKeyFunc, - "tasks[*].depends_on": depKeyFunc, - } - tests := []struct { name string a, b Job @@ -717,19 +766,19 @@ func TestGetStructDiffNestedDependsOn(t *testing.T) { name: "depends_on field change", a: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a", Outcome: "success"}}}}}, b: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a", Outcome: "failed"}}}}}, - want: []ResolvedChange{{Field: "tasks[task_key='c'].depends_on[task_key='a'].outcome", Old: "success", New: "failed"}}, + want: []ResolvedChange{{Field: "tasks[='c'].depends_on[='a'].outcome", Old: "success", New: "failed"}}, }, { name: "depends_on element added", a: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}}}}}, b: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}, {TaskKey: "b"}}}}}, - want: []ResolvedChange{{Field: "tasks[task_key='c'].depends_on[task_key='b']", Old: nil, New: Dep{TaskKey: "b"}}}, + want: []ResolvedChange{{Field: "tasks[='c'].depends_on[='b']", Old: nil, New: Dep{TaskKey: "b"}}}, }, { name: "depends_on element removed", a: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}, {TaskKey: "b"}}}}}, b: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}}}}}, - want: []ResolvedChange{{Field: "tasks[task_key='c'].depends_on[task_key='b']", Old: Dep{TaskKey: "b"}, New: nil}}, + want: []ResolvedChange{{Field: "tasks[='c'].depends_on[='b']", Old: Dep{TaskKey: "b"}, New: nil}}, }, { name: "tasks and depends_on both reordered no diff", @@ -747,7 +796,7 @@ func TestGetStructDiffNestedDependsOn(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, sliceKeys) + got, err := GetStructDiff(tt.a, tt.b) assert.NoError(t, err) assert.Equal(t, tt.want, resolveChanges(got)) }) @@ -767,15 +816,7 @@ type Root struct { Nested []Nested `json:"nested,omitempty"` } -func itemKeyFunc(item Item) (string, string) { - return "id", item.ID -} - func TestGetStructDiffNestedSliceKeys(t *testing.T) { - sliceKeys := map[string]KeyFunc{ - "nested[*].items": itemKeyFunc, - } - tests := []struct { name string a, b any @@ -791,92 +832,26 @@ func TestGetStructDiffNestedSliceKeys(t *testing.T) { name: "nested slice field change", a: Root{Nested: []Nested{{Items: []Item{{ID: "x", Value: 1}}}}}, b: Root{Nested: []Nested{{Items: []Item{{ID: "x", Value: 99}}}}}, - want: []ResolvedChange{{Field: "nested[0].items[id='x'].value", Old: 1, New: 99}}, + want: []ResolvedChange{{Field: "nested[0].items[='x'].value", Old: 1, New: 99}}, }, { name: "nested slice element added", a: Root{Nested: []Nested{{Items: []Item{{ID: "x", Value: 1}}}}}, b: Root{Nested: []Nested{{Items: []Item{{ID: "x", Value: 1}, {ID: "y", Value: 2}}}}}, - want: []ResolvedChange{{Field: "nested[0].items[id='y']", Old: nil, New: Item{ID: "y", Value: 2}}}, + want: []ResolvedChange{{Field: "nested[0].items[='y']", Old: nil, New: Item{ID: "y", Value: 2}}}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, sliceKeys) + got, err := GetStructDiff(tt.a, tt.b) assert.NoError(t, err) assert.Equal(t, tt.want, resolveChanges(got)) }) } } -func TestGetStructDiffSliceKeysInvalidFunc(t *testing.T) { - tests := []struct { - name string - keyFunc any - errMsg string - }{ - { - name: "not a function", - keyFunc: "not a function", - errMsg: "KeyFunc must be a function, got string", - }, - { - name: "wrong number of parameters", - keyFunc: func() (string, string) { return "", "" }, - errMsg: "KeyFunc must have exactly 1 parameter, got 0", - }, - { - name: "too many parameters", - keyFunc: func(a, b Task) (string, string) { return "", "" }, - errMsg: "KeyFunc must have exactly 1 parameter, got 2", - }, - { - name: "wrong number of returns", - keyFunc: func(t Task) string { return "" }, - errMsg: "KeyFunc must return exactly 2 values, got 1", - }, - { - name: "wrong first return type", - keyFunc: func(t Task) (int, string) { return 0, "" }, - errMsg: "KeyFunc must return (string, string), got (int, string)", - }, - { - name: "wrong second return type", - keyFunc: func(t Task) (string, int) { return "", 0 }, - errMsg: "KeyFunc must return (string, string), got (string, int)", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - sliceKeys := map[string]KeyFunc{"tasks": tt.keyFunc} - a := Job{Tasks: []Task{{TaskKey: "a"}}} - b := Job{Tasks: []Task{{TaskKey: "a"}}} - _, err := GetStructDiff(a, b, sliceKeys) - assert.EqualError(t, err, tt.errMsg) - }) - } -} - -func TestGetStructDiffSliceKeysWrongArgType(t *testing.T) { - // Function expects Item but slice contains Task - sliceKeys := map[string]KeyFunc{ - "tasks": func(item Item) (string, string) { - return "id", item.ID - }, - } - a := Job{Tasks: []Task{{TaskKey: "a"}}} - b := Job{Tasks: []Task{{TaskKey: "b"}}} - _, err := GetStructDiff(a, b, sliceKeys) - assert.EqualError(t, err, "KeyFunc expects structdiff.Item, got structdiff.Task") -} - func TestGetStructDiffSliceKeysDuplicates(t *testing.T) { - sliceKeys := map[string]KeyFunc{ - "tasks": taskKeyFunc, - } - tests := []struct { name string a, b Job @@ -892,34 +867,34 @@ func TestGetStructDiffSliceKeysDuplicates(t *testing.T) { name: "duplicates with field change", a: Job{Tasks: []Task{{TaskKey: "a", Description: "1"}, {TaskKey: "a", Description: "2"}}}, b: Job{Tasks: []Task{{TaskKey: "a", Description: "1"}, {TaskKey: "a", Description: "changed"}}}, - want: []ResolvedChange{{Field: "tasks[task_key='a'].description", Old: "2", New: "changed"}}, + want: []ResolvedChange{{Field: "tasks[='a'].description", Old: "2", New: "changed"}}, }, { name: "extra in old is deleted", a: Job{Tasks: []Task{{TaskKey: "a", Description: "1"}, {TaskKey: "a", Description: "2"}}}, b: Job{Tasks: []Task{{TaskKey: "a", Description: "1"}}}, - want: []ResolvedChange{{Field: "tasks[task_key='a']", Old: Task{TaskKey: "a", Description: "2"}, New: nil}}, + want: []ResolvedChange{{Field: "tasks[='a']", Old: Task{TaskKey: "a", Description: "2"}, New: nil}}, }, { name: "extra in new is added", a: Job{Tasks: []Task{{TaskKey: "a", Description: "1"}}}, b: Job{Tasks: []Task{{TaskKey: "a", Description: "1"}, {TaskKey: "a", Description: "2"}}}, - want: []ResolvedChange{{Field: "tasks[task_key='a']", Old: nil, New: Task{TaskKey: "a", Description: "2"}}}, + want: []ResolvedChange{{Field: "tasks[='a']", Old: nil, New: Task{TaskKey: "a", Description: "2"}}}, }, { name: "two in old one in new with change", a: Job{Tasks: []Task{{TaskKey: "a", Description: "1"}, {TaskKey: "a", Description: "2"}}}, b: Job{Tasks: []Task{{TaskKey: "a", Description: "changed"}}}, want: []ResolvedChange{ - {Field: "tasks[task_key='a'].description", Old: "1", New: "changed"}, - {Field: "tasks[task_key='a']", Old: Task{TaskKey: "a", Description: "2"}, New: nil}, + {Field: "tasks[='a'].description", Old: "1", New: "changed"}, + {Field: "tasks[='a']", Old: Task{TaskKey: "a", Description: "2"}, New: nil}, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, sliceKeys) + got, err := GetStructDiff(tt.a, tt.b) assert.NoError(t, err) assert.Equal(t, tt.want, resolveChanges(got)) }) @@ -946,7 +921,7 @@ func TestDiffTaggedEmbedIsReportedUnderItsName(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{"leaf":{"value":"after"},"own":"o"}`, string(blob)) - changes, err := GetStructDiff(before, after, nil) + changes, err := GetStructDiff(before, after) require.NoError(t, err) require.Len(t, changes, 1) assert.Equal(t, "leaf.value", changes[0].Path.String()) @@ -969,7 +944,7 @@ func TestGetStructDiffNamedStringMapKey(t *testing.T) { a := namedKeyMapHolder{M: map[namedMapKey]string{"pre": "a", "post": "x"}} b := namedKeyMapHolder{M: map[namedMapKey]string{"pre": "b", "post": "x"}} - got, err := GetStructDiff(&a, &b, nil) + got, err := GetStructDiff(&a, &b) assert.NoError(t, err) assert.Equal(t, []ResolvedChange{{Field: "m['pre']", Old: "a", New: "b"}}, resolveChanges(got)) } @@ -1028,7 +1003,7 @@ func TestGetStructDiffWholeBlock(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetStructDiff(tt.a, tt.b, nil) + got, err := GetStructDiff(tt.a, tt.b) assert.NoError(t, err) assert.Equal(t, tt.want, resolveChanges(got)) }) diff --git a/libs/structs/structdiff/jobsettings_test.go b/libs/structs/structdiff/jobsettings_test.go index f5de67503a0..754ef309308 100644 --- a/libs/structs/structdiff/jobsettings_test.go +++ b/libs/structs/structdiff/jobsettings_test.go @@ -444,16 +444,16 @@ const jobExampleResponseNils = ` func testEqual(t *testing.T, input string) { var x, y jobs.JobSettings require.NoError(t, json.Unmarshal([]byte(input), &x)) - changes, err := GetStructDiff(x, x, nil) + changes, err := GetStructDiff(x, x) require.NoError(t, err) require.Nil(t, changes) require.NoError(t, json.Unmarshal([]byte(input), &y)) - changes, err = GetStructDiff(x, y, nil) + changes, err = GetStructDiff(x, y) require.NoError(t, err) require.Nil(t, changes) - changes, err = GetStructDiff(&x, &y, nil) + changes, err = GetStructDiff(&x, &y) require.NoError(t, err) require.Nil(t, changes) } @@ -471,7 +471,7 @@ func TestJobDiff(t *testing.T) { require.NoError(t, json.Unmarshal([]byte(jobExampleResponseZeroes), &zero)) require.NoError(t, json.Unmarshal([]byte(jobExampleResponseNils), &nils)) - changes, err := GetStructDiff(job, zero, nil) + changes, err := GetStructDiff(job, zero) require.NoError(t, err) require.GreaterOrEqual(t, len(changes), 75) assert.Equal(t, "budget_policy_id", changes[0].Path.String()) @@ -488,7 +488,7 @@ func TestJobDiff(t *testing.T) { assert.Equal(t, "string", changes[3].Old) assert.Empty(t, changes[3].New) - changes, err = GetStructDiff(job, nils, nil) + changes, err = GetStructDiff(job, nils) require.NoError(t, err) require.GreaterOrEqual(t, len(changes), 77) assert.Equal(t, "budget_policy_id", changes[0].Path.String()) @@ -509,7 +509,7 @@ func TestJobDiff(t *testing.T) { assert.Equal(t, "string", changes[3].Old) assert.Nil(t, changes[3].New) - changes, err = GetStructDiff(zero, nils, nil) + changes, err = GetStructDiff(zero, nils) require.NoError(t, err) assert.GreaterOrEqual(t, len(changes), 58) assert.Equal(t, "budget_policy_id", changes[0].Path.String()) diff --git a/libs/structs/structdiff/opaque_test.go b/libs/structs/structdiff/opaque_test.go index 1ec4cb89abe..2828031a9e3 100644 --- a/libs/structs/structdiff/opaque_test.go +++ b/libs/structs/structdiff/opaque_test.go @@ -63,7 +63,7 @@ func TestGetStructDiffOpaqueAtRoot(t *testing.T) { {"value", *a, *b}, } { t.Run(tt.name, func(t *testing.T) { - changes, err := GetStructDiff(tt.a, tt.b, nil) + changes, err := GetStructDiff(tt.a, tt.b) require.NoError(t, err) require.Len(t, changes, 1) assert.Empty(t, changes[0].Path.String()) diff --git a/libs/structs/structpath/path.go b/libs/structs/structpath/path.go index 92715dc09d6..150cde61a74 100644 --- a/libs/structs/structpath/path.go +++ b/libs/structs/structpath/path.go @@ -466,6 +466,11 @@ func parse(s string, wildcardAllowed bool) (*PatternNode, error) { if ch >= '0' && ch <= '9' { currentToken.WriteByte(ch) state = stateIndex + } else if ch == '=' { + // Field-agnostic keyed element: [='value']. The key field is omitted; + // resolution is by value (the resolver has the type to match it). + keyValueKey = "" + state = stateKeyValueEquals } else if ch == '\'' { state = stateMapKey } else if ch == '*' { diff --git a/libs/structs/structpath/path_test.go b/libs/structs/structpath/path_test.go index b8623db76d0..75b3f4cae2a 100644 --- a/libs/structs/structpath/path_test.go +++ b/libs/structs/structpath/path_test.go @@ -51,6 +51,13 @@ func TestPathAndPatternNode(t *testing.T) { String: "[name='foo']", KeyValue: []string{"name", "foo"}, }, + { + name: "field-agnostic key value", + pathNode: NewKeyValue(nil, "", "foo"), + patternNode: NewPatternKeyValue(nil, "", "foo"), + String: "[='foo']", + KeyValue: []string{"", "foo"}, + }, // Two node tests { diff --git a/libs/testserver/permissions.go b/libs/testserver/permissions.go index a3584c1d49a..ab13d167721 100644 --- a/libs/testserver/permissions.go +++ b/libs/testserver/permissions.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/databricks/databricks-sdk-go/service/iam" + "github.com/google/uuid" ) // source: https://github.com/databricks/terraform-provider-databricks/blob/main/permissions/permission_definitions.go @@ -39,6 +40,13 @@ var requestObjectTypeToObjectType = map[string]string{ "alertsv2": "alertv2", } +// isServicePrincipalID reports whether name is a service principal application ID +// (a UUID). The Permissions API treats a UUID-valued user_name as a service principal. +func isServicePrincipalID(name string) bool { + _, err := uuid.Parse(name) + return err == nil +} + // aclPrincipalKey returns a unique key identifying the principal in an ACL entry. func aclPrincipalKey(acl iam.AccessControlResponse) string { switch { @@ -269,15 +277,27 @@ func (s *FakeWorkspace) SetPermissions(req Request) any { // Convert AccessControlRequest to AccessControlResponse and replace the ACL. existingPermissions.AccessControlList = nil for _, acl := range updateRequest.AccessControlList { - display := acl.UserName + userName := acl.UserName + servicePrincipalName := acl.ServicePrincipalName + + // The real Permissions API resolves a user_name that is actually a service + // principal's application ID (a UUID) to a service principal, and returns it + // as service_principal_name on GET. Model that here so a bundle that declares + // a service principal under user_name converges the same way it does on cloud. + if userName != "" && isServicePrincipalID(userName) { + servicePrincipalName = userName + userName = "" + } + + display := userName if display == "" { - display = acl.ServicePrincipalName + display = servicePrincipalName } response := iam.AccessControlResponse{ - UserName: acl.UserName, + UserName: userName, GroupName: acl.GroupName, - ServicePrincipalName: acl.ServicePrincipalName, + ServicePrincipalName: servicePrincipalName, DisplayName: display, }