diff --git a/.nextchanges/bundles/apps-user-api-scopes.md b/.nextchanges/bundles/apps-user-api-scopes.md new file mode 100644 index 00000000000..ffe71034788 --- /dev/null +++ b/.nextchanges/bundles/apps-user-api-scopes.md @@ -0,0 +1 @@ +* Do not send an app's `user_api_scopes` in update requests unless the bundle declares or removes them, so scopes added outside the bundle are not cleared. ([#7193](https://github.com/databricks/cli/pull/7193)) diff --git a/acceptance/bundle/resources/apps/lifecycle-started/output.txt b/acceptance/bundle/resources/apps/lifecycle-started/output.txt index 05c88f1cd78..799ea1aaec0 100644 --- a/acceptance/bundle/resources/apps/lifecycle-started/output.txt +++ b/acceptance/bundle/resources/apps/lifecycle-started/output.txt @@ -90,7 +90,7 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged "description": "MY_APP_DESCRIPTION_2", "name": "[UNIQUE_NAME]" }, - "update_mask": "description,budget_policy_id,usage_policy_id,resources,user_api_scopes,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" + "update_mask": "description,budget_policy_id,usage_policy_id,resources,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" } } @@ -117,7 +117,7 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged "description": "MY_APP_DESCRIPTION_3", "name": "[UNIQUE_NAME]" }, - "update_mask": "description,budget_policy_id,usage_policy_id,resources,user_api_scopes,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" + "update_mask": "description,budget_policy_id,usage_policy_id,resources,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" } } { diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/app/app.py b/acceptance/bundle/resources/apps/update-declared-scopes/app/app.py new file mode 100644 index 00000000000..f1a18139c84 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/app/app.py @@ -0,0 +1 @@ +print("Hello world!") diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/databricks.yml b/acceptance/bundle/resources/apps/update-declared-scopes/databricks.yml new file mode 100644 index 00000000000..ba2dca0ddfc --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/databricks.yml @@ -0,0 +1,10 @@ +bundle: + name: test-bundle + +resources: + apps: + mykey: + name: myappname + description: my_app_description + source_code_path: ./app + user_api_scopes: [sql, files.files] diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/out.description.requests.json b/acceptance/bundle/resources/apps/update-declared-scopes/out.description.requests.json new file mode 100644 index 00000000000..16d08070af8 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/out.description.requests.json @@ -0,0 +1,15 @@ +{ + "method": "POST", + "path": "/api/2.0/apps/myappname/update", + "body": { + "app": { + "description": "MY_APP_DESCRIPTION", + "name": "myappname", + "user_api_scopes": [ + "sql", + "files.files" + ] + }, + "update_mask": "description,budget_policy_id,usage_policy_id,resources,user_api_scopes,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" + } +} diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/out.scopes.requests.json b/acceptance/bundle/resources/apps/update-declared-scopes/out.scopes.requests.json new file mode 100644 index 00000000000..40a2649e466 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/out.scopes.requests.json @@ -0,0 +1,15 @@ +{ + "method": "POST", + "path": "/api/2.0/apps/myappname/update", + "body": { + "app": { + "description": "MY_APP_DESCRIPTION", + "name": "myappname", + "user_api_scopes": [ + "sql", + "iam.access-control:read" + ] + }, + "update_mask": "description,budget_policy_id,usage_policy_id,resources,user_api_scopes,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" + } +} diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/out.test.toml b/acceptance/bundle/resources/apps/update-declared-scopes/out.test.toml new file mode 100644 index 00000000000..a927a5fbc06 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/output.txt b/acceptance/bundle/resources/apps/update-declared-scopes/output.txt new file mode 100644 index 00000000000..efb4cabe0a0 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/output.txt @@ -0,0 +1,65 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Created apps.mykey +Files: 2 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 0 unchanged + +=== Out-of-band: add a scope the bundle cannot declare (e.g. via account-level update) +>>> MSYS_NO_PATHCONV=1 [CLI] api post /api/2.0/apps/myappname/update --json {"app": {"name": "myappname", "user_api_scopes": ["sql", "files.files", "jobs"]}, "update_mask": "user_api_scopes"} + +>>> [CLI] apps get myappname +[ + "sql", + "files.files", + "jobs" +] + +=== Plan after out-of-band change +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + +=== Change description and re-deploy; declared scopes are in the mask, so the remote value is replaced +>>> update_file.py databricks.yml my_app_description MY_APP_DESCRIPTION + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Updated apps.mykey +Files: 1 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged + +>>> print_requests + +>>> [CLI] apps get myappname +{ + "description": "MY_APP_DESCRIPTION", + "user_api_scopes": [ + "sql", + "files.files" + ] +} + +=== Change the declared scopes +>>> update_file.py databricks.yml [sql, files.files] [sql, iam.access-control:read] + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Updated apps.mykey +Files: 2 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged + +>>> print_requests + +>>> [CLI] apps get myappname +[ + "sql", + "iam.access-control:read" +] + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.apps.mykey + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/script b/acceptance/bundle/resources/apps/update-declared-scopes/script new file mode 100644 index 00000000000..ae85a807d93 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/script @@ -0,0 +1,28 @@ +print_requests() { + print_requests.py //apps '^//api/2.0/bundle' --del-body url +} + +trace $CLI bundle deploy +rm "$OUT_REQUESTS" + +title "Out-of-band: add a scope the bundle cannot declare (e.g. via account-level update)" +trace MSYS_NO_PATHCONV=1 $CLI api post /api/2.0/apps/myappname/update --json '{"app": {"name": "myappname", "user_api_scopes": ["sql", "files.files", "jobs"]}, "update_mask": "user_api_scopes"}' > /dev/null +trace $CLI apps get myappname | jq .user_api_scopes +rm "$OUT_REQUESTS" + +title "Plan after out-of-band change" +trace $CLI bundle plan + +title "Change description and re-deploy; declared scopes are in the mask, so the remote value is replaced" +trace update_file.py databricks.yml my_app_description MY_APP_DESCRIPTION +trace $CLI bundle deploy +trace print_requests > out.description.requests.json +trace $CLI apps get myappname | jq '{description, user_api_scopes}' + +title "Change the declared scopes" +trace update_file.py databricks.yml "[sql, files.files]" "[sql, iam.access-control:read]" +trace $CLI bundle deploy +trace print_requests > out.scopes.requests.json +trace $CLI apps get myappname | jq .user_api_scopes + +trace $CLI bundle destroy --auto-approve diff --git a/acceptance/bundle/resources/apps/update-declared-scopes/test.toml b/acceptance/bundle/resources/apps/update-declared-scopes/test.toml new file mode 100644 index 00000000000..474ccd75b48 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-declared-scopes/test.toml @@ -0,0 +1,2 @@ +Cloud = false # jobs scope can only be added with account-admin privileges +Ignore = [".databricks", "databricks.yml"] diff --git a/acceptance/bundle/resources/apps/update-omitted-scopes/app/app.py b/acceptance/bundle/resources/apps/update-omitted-scopes/app/app.py new file mode 100644 index 00000000000..f1a18139c84 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-omitted-scopes/app/app.py @@ -0,0 +1 @@ +print("Hello world!") diff --git a/acceptance/bundle/resources/apps/update-omitted-scopes/databricks.yml b/acceptance/bundle/resources/apps/update-omitted-scopes/databricks.yml new file mode 100644 index 00000000000..6a74ae8e35a --- /dev/null +++ b/acceptance/bundle/resources/apps/update-omitted-scopes/databricks.yml @@ -0,0 +1,9 @@ +bundle: + name: test-bundle + +resources: + apps: + mykey: + name: myappname + description: my_app_description + source_code_path: ./app diff --git a/acceptance/bundle/resources/apps/update-omitted-scopes/out.deploy.requests.json b/acceptance/bundle/resources/apps/update-omitted-scopes/out.deploy.requests.json new file mode 100644 index 00000000000..a7255ff62c5 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-omitted-scopes/out.deploy.requests.json @@ -0,0 +1,11 @@ +{ + "method": "POST", + "path": "/api/2.0/apps/myappname/update", + "body": { + "app": { + "description": "MY_APP_DESCRIPTION", + "name": "myappname" + }, + "update_mask": "description,budget_policy_id,usage_policy_id,resources,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" + } +} diff --git a/acceptance/bundle/resources/apps/update-omitted-scopes/out.test.toml b/acceptance/bundle/resources/apps/update-omitted-scopes/out.test.toml new file mode 100644 index 00000000000..a927a5fbc06 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-omitted-scopes/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/apps/update-omitted-scopes/output.txt b/acceptance/bundle/resources/apps/update-omitted-scopes/output.txt new file mode 100644 index 00000000000..131664e6e3f --- /dev/null +++ b/acceptance/bundle/resources/apps/update-omitted-scopes/output.txt @@ -0,0 +1,37 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Created apps.mykey +Files: 2 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 0 unchanged + +=== Out-of-band: set scopes on an app whose bundle does not declare any +>>> MSYS_NO_PATHCONV=1 [CLI] api post /api/2.0/apps/myappname/update --json {"app": {"name": "myappname", "user_api_scopes": ["sql", "jobs"]}, "update_mask": "user_api_scopes"} + +=== Change description and re-deploy; user_api_scopes must not be in the update mask +>>> update_file.py databricks.yml my_app_description MY_APP_DESCRIPTION + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Updated apps.mykey +Files: 1 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged + +>>> print_requests + +>>> [CLI] apps get myappname +{ + "description": "MY_APP_DESCRIPTION", + "user_api_scopes": [ + "sql", + "jobs" + ] +} + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.apps.mykey + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/resources/apps/update-omitted-scopes/script b/acceptance/bundle/resources/apps/update-omitted-scopes/script new file mode 100644 index 00000000000..db205b2fa4b --- /dev/null +++ b/acceptance/bundle/resources/apps/update-omitted-scopes/script @@ -0,0 +1,18 @@ +print_requests() { + print_requests.py //apps '^//api/2.0/bundle' --del-body url +} + +trace $CLI bundle deploy +rm "$OUT_REQUESTS" + +title "Out-of-band: set scopes on an app whose bundle does not declare any" +trace MSYS_NO_PATHCONV=1 $CLI api post /api/2.0/apps/myappname/update --json '{"app": {"name": "myappname", "user_api_scopes": ["sql", "jobs"]}, "update_mask": "user_api_scopes"}' > /dev/null +rm "$OUT_REQUESTS" + +title "Change description and re-deploy; user_api_scopes must not be in the update mask" +trace update_file.py databricks.yml my_app_description MY_APP_DESCRIPTION +trace $CLI bundle deploy +trace print_requests > out.deploy.requests.json +trace $CLI apps get myappname | jq '{description, user_api_scopes}' + +trace $CLI bundle destroy --auto-approve diff --git a/acceptance/bundle/resources/apps/update-omitted-scopes/test.toml b/acceptance/bundle/resources/apps/update-omitted-scopes/test.toml new file mode 100644 index 00000000000..474ccd75b48 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-omitted-scopes/test.toml @@ -0,0 +1,2 @@ +Cloud = false # jobs scope can only be added with account-admin privileges +Ignore = [".databricks", "databricks.yml"] diff --git a/acceptance/bundle/resources/apps/update-removed-scopes/app/app.py b/acceptance/bundle/resources/apps/update-removed-scopes/app/app.py new file mode 100644 index 00000000000..f1a18139c84 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-removed-scopes/app/app.py @@ -0,0 +1 @@ +print("Hello world!") diff --git a/acceptance/bundle/resources/apps/update-removed-scopes/databricks.yml b/acceptance/bundle/resources/apps/update-removed-scopes/databricks.yml new file mode 100644 index 00000000000..ba2dca0ddfc --- /dev/null +++ b/acceptance/bundle/resources/apps/update-removed-scopes/databricks.yml @@ -0,0 +1,10 @@ +bundle: + name: test-bundle + +resources: + apps: + mykey: + name: myappname + description: my_app_description + source_code_path: ./app + user_api_scopes: [sql, files.files] diff --git a/acceptance/bundle/resources/apps/update-removed-scopes/out.deploy.requests.json b/acceptance/bundle/resources/apps/update-removed-scopes/out.deploy.requests.json new file mode 100644 index 00000000000..77a445c3149 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-removed-scopes/out.deploy.requests.json @@ -0,0 +1,11 @@ +{ + "method": "POST", + "path": "/api/2.0/apps/myappname/update", + "body": { + "app": { + "description": "my_app_description", + "name": "myappname" + }, + "update_mask": "description,budget_policy_id,usage_policy_id,resources,user_api_scopes,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" + } +} diff --git a/acceptance/bundle/resources/apps/update-removed-scopes/out.test.toml b/acceptance/bundle/resources/apps/update-removed-scopes/out.test.toml new file mode 100644 index 00000000000..a927a5fbc06 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-removed-scopes/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/apps/update-removed-scopes/output.txt b/acceptance/bundle/resources/apps/update-removed-scopes/output.txt new file mode 100644 index 00000000000..d58963a335f --- /dev/null +++ b/acceptance/bundle/resources/apps/update-removed-scopes/output.txt @@ -0,0 +1,33 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Created apps.mykey +Files: 2 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 0 unchanged + +=== Remove user_api_scopes from the config; the mask must include it with no value in the body +>>> update_file.py databricks.yml user_api_scopes: [sql, files.files] + +>>> [CLI] bundle plan +update apps.mykey + +Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Updated apps.mykey +Files: 1 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged + +>>> print_requests + +>>> [CLI] apps get myappname +null + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.apps.mykey + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/resources/apps/update-removed-scopes/script b/acceptance/bundle/resources/apps/update-removed-scopes/script new file mode 100644 index 00000000000..ef2f52d302d --- /dev/null +++ b/acceptance/bundle/resources/apps/update-removed-scopes/script @@ -0,0 +1,15 @@ +print_requests() { + print_requests.py //apps '^//api/2.0/bundle' --del-body url +} + +trace $CLI bundle deploy +rm "$OUT_REQUESTS" + +title "Remove user_api_scopes from the config; the mask must include it with no value in the body" +trace update_file.py databricks.yml "user_api_scopes: [sql, files.files]" "" +trace $CLI bundle plan +trace $CLI bundle deploy +trace print_requests > out.deploy.requests.json +trace $CLI apps get myappname | jq .user_api_scopes + +trace $CLI bundle destroy --auto-approve diff --git a/acceptance/bundle/resources/apps/update-removed-scopes/test.toml b/acceptance/bundle/resources/apps/update-removed-scopes/test.toml new file mode 100644 index 00000000000..00495bf9c14 --- /dev/null +++ b/acceptance/bundle/resources/apps/update-removed-scopes/test.toml @@ -0,0 +1,2 @@ +Cloud = false +Ignore = [".databricks", "databricks.yml"] diff --git a/acceptance/bundle/resources/apps/update/out.requests.json b/acceptance/bundle/resources/apps/update/out.requests.json index e68269ec514..b3b05c6e140 100644 --- a/acceptance/bundle/resources/apps/update/out.requests.json +++ b/acceptance/bundle/resources/apps/update/out.requests.json @@ -17,7 +17,7 @@ "description": "MY_APP_DESCRIPTION", "name": "myappname" }, - "update_mask": "description,budget_policy_id,usage_policy_id,resources,user_api_scopes,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" + "update_mask": "description,budget_policy_id,usage_policy_id,resources,compute_size,compute_min_instances,compute_max_instances,git_repository,telemetry_export_destinations" } } { diff --git a/bundle/direct/dresources/app.go b/bundle/direct/dresources/app.go index 22cd7cc2b4e..a7042b0c776 100644 --- a/bundle/direct/dresources/app.go +++ b/bundle/direct/dresources/app.go @@ -176,17 +176,31 @@ var UpdateMaskFields = []string{ "telemetry_export_destinations", } +const userApiScopes = "user_api_scopes" + var updateMask = strings.Join(UpdateMaskFields, ",") +// updateMaskWithoutScopes is updateMask minus user_api_scopes. +var updateMaskWithoutScopes = strings.Join(slices.DeleteFunc(slices.Clone(UpdateMaskFields), func(f string) bool { + return f == userApiScopes +}), ",") + func (r *ResourceApp) DoUpdate(ctx context.Context, id string, config *AppState, entry *PlanEntry) (*AppRemote, error) { // Deploy-only fields (source_code_path, config, git_source, lifecycle) are excluded // from the request body; see appRequestBody. if hasAppChanges(entry) { app := appRequestBody(config) + // Sending user_api_scopes in the mask clears it unless the body carries a value, and some + // scopes (e.g. "jobs") can only be added outside this API. So leave it out unless the + // bundle manages it: declares it, or removed it (the mask is what clears it). + mask := updateMaskWithoutScopes + if change := entry.Changes[userApiScopes]; len(config.UserApiScopes) > 0 || (change != nil && change.Action != deployplan.Skip) { + mask = updateMask + } request := apps.AsyncUpdateAppRequest{ App: &app, AppName: id, - UpdateMask: updateMask, + UpdateMask: mask, } updateWaiter, err := r.client.Apps.CreateUpdate(ctx, request) if err != nil { diff --git a/bundle/direct/dresources/configs/apps.yml b/bundle/direct/dresources/configs/apps.yml index a6725316bec..1a86a6de78d 100644 --- a/bundle/direct/dresources/configs/apps.yml +++ b/bundle/direct/dresources/configs/apps.yml @@ -16,5 +16,9 @@ backend_defaults: - field: lifecycle - field: lifecycle.started ignore_remote_changes: + # Scopes can be added out-of-band (e.g. "jobs" via account-level custom-app-integration update, + # which a bundle cannot declare). Remote-only drift alone does not plan an update. + - field: user_api_scopes + reason: managed - field: space # This field is not yet supported by Update APIs but exposed in the API spec. TODO: fix when update APIs supports it. reason: managed diff --git a/libs/testserver/apps.go b/libs/testserver/apps.go index c70115d060f..d6f302c4404 100644 --- a/libs/testserver/apps.go +++ b/libs/testserver/apps.go @@ -86,8 +86,12 @@ func (s *FakeWorkspace) AppsCreateUpdate(req Request, name string) Response { } for field := range strings.SplitSeq(updateReq.UpdateMask, ",") { - if v, ok := updateMap[strings.TrimSpace(field)]; ok { - existingMap[strings.TrimSpace(field)] = v + field = strings.TrimSpace(field) + // Like the real API, a masked field absent from the body is cleared. + if v, ok := updateMap[field]; ok { + existingMap[field] = v + } else { + delete(existingMap, field) } } @@ -95,9 +99,12 @@ func (s *FakeWorkspace) AppsCreateUpdate(req Request, name string) Response { if err != nil { return Response{Body: fmt.Sprintf("internal error: %s", err), StatusCode: http.StatusInternalServerError} } - if err := json.Unmarshal(merged, &existing); err != nil { + // Unmarshal into a fresh struct so that cleared fields are not retained. + var updated apps.App + if err := json.Unmarshal(merged, &updated); err != nil { return Response{Body: fmt.Sprintf("internal error: %s", err), StatusCode: http.StatusInternalServerError} } + existing = updated } setUcSecurableKinds(&existing) s.Apps[name] = existing