From d488e52027e2a8a5be9afeced11cdef93e60a8a1 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 21 Aug 2026 18:13:57 +0200 Subject: [PATCH 01/16] Reproduce UpdateSchema "Nothing to update" on a comment set outside the bundle A schema without `comment` in the config becomes undeployable once someone sets a description on it in UC: the engine reads the remote comment, plans an update, and every field of the PATCH serializes away under omitempty. UC rejects the empty body with `400 INVALID_PARAMETER_VALUE / UpdateSchema Nothing to update`, which aborts the whole deploy. Add an acceptance test for it and make the fake workspace reject an empty UpdateSchema payload the way UC does, so the local run fails the same way the cloud one does. Co-authored-by: Isaac --- .../comment_out_of_band/databricks.yml.tmpl | 8 ++ .../drift/comment_out_of_band/out.test.toml | 3 + .../drift/comment_out_of_band/output.txt | 78 +++++++++++++++++++ .../schemas/drift/comment_out_of_band/script | 32 ++++++++ .../drift/comment_out_of_band/test.toml | 11 +++ libs/testserver/schemas.go | 33 ++++++++ 6 files changed, 165 insertions(+) create mode 100644 acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml create mode 100644 acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt create mode 100644 acceptance/bundle/resources/schemas/drift/comment_out_of_band/script create mode 100644 acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl new file mode 100644 index 00000000000..e3283c6b3bf --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl @@ -0,0 +1,8 @@ +bundle: + name: test-bundle + +resources: + schemas: + schema1: + catalog_name: main + name: test-schema-$UNIQUE_NAME diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml new file mode 100644 index 00000000000..8c52d40aa2d --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt new file mode 100644 index 00000000000..be6797d19ec --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt @@ -0,0 +1,78 @@ + +=== Deploy a schema that does not declare a comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Created schemas.schema1 +Files: 0 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 0 unchanged + +=== Set the comment out of band, the way Catalog Explorer does +=== The remote comment is drift, so the plan updates the schema +>>> [CLI] bundle plan --output json +{ + "action": "update", + "remote": "set outside the bundle" +} + +=== Redeploy clears the comment +>>> errcode [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Error: cannot update resources.schemas.schema1: updating id=main.test-schema-[UNIQUE_NAME]: UpdateSchema Nothing to update. (400 INVALID_PARAMETER_VALUE) + +Endpoint: PATCH [DATABRICKS_URL]/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] +HTTP Status: 400 Bad Request +API error_code: INVALID_PARAMETER_VALUE +API message: UpdateSchema Nothing to update. + +Files: 0 uploaded, 0 deleted + +Exit code: 1 + +=== The comment is gone +>>> [CLI] api get /api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] +"set outside the bundle" + +=== Redeploy is a no-op +>>> errcode [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Error: cannot update resources.schemas.schema1: updating id=main.test-schema-[UNIQUE_NAME]: UpdateSchema Nothing to update. (400 INVALID_PARAMETER_VALUE) + +Endpoint: PATCH [DATABRICKS_URL]/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] +HTTP Status: 400 Bad Request +API error_code: INVALID_PARAMETER_VALUE +API message: UpdateSchema Nothing to update. + +Files: 0 uploaded, 0 deleted + +Exit code: 1 + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.schema1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Destroy: 1 deleted + +=== The UpdateSchema payloads the deploys sent +>>> print_requests.py --method PATCH //unity-catalog/schemas +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", + "body": { + "comment": "set outside the bundle" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", + "body": {} +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", + "body": {} +} diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script new file mode 100644 index 00000000000..dac79d7d8d5 --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script @@ -0,0 +1,32 @@ +echo "*" > .gitignore +envsubst < databricks.yml.tmpl > databricks.yml + +SCHEMA="main.test-schema-${UNIQUE_NAME}" + +# print_requests.py consumes out.requests.txt, so it has to run after the last +# request the test makes, which is the destroy below. +cleanup() { + trace $CLI bundle destroy --auto-approve + + title "The UpdateSchema payloads the deploys sent" + trace print_requests.py --method PATCH //unity-catalog/schemas +} +trap cleanup EXIT + +title "Deploy a schema that does not declare a comment" +trace $CLI bundle deploy + +title "Set the comment out of band, the way Catalog Explorer does" +MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/schemas/$SCHEMA" --json '{"comment":"set outside the bundle"}' > /dev/null + +title "The remote comment is drift, so the plan updates the schema" +trace $CLI bundle plan --output json | jq '.plan[].changes.comment' + +title "Redeploy clears the comment" +trace errcode $CLI bundle deploy + +title "The comment is gone" +MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | jq '.comment' + +title "Redeploy is a no-op" +trace errcode $CLI bundle deploy diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml new file mode 100644 index 00000000000..4f7503fc8e8 --- /dev/null +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml @@ -0,0 +1,11 @@ +Cloud = true +RequiresUnityCatalog = true + +# UpdateSchema payload construction is direct-engine-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", +] diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index 66d23a3011c..f685239a62f 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -80,6 +80,28 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { } } + var fields map[string]json.RawMessage + + if err := json.Unmarshal(req.Body, &fields); err != nil { + return Response{ + Body: fmt.Sprintf("internal error: %s", err), + StatusCode: http.StatusInternalServerError, + } + } + + // UC rejects a PATCH that carries no field to update instead of treating it as a + // no-op. Verified against a real workspace: {} and {"comment": null} are rejected, + // while {"comment": ""} and {"custom_max_retention_hours": 0} are accepted. + if !hasFieldToUpdate(fields) { + return Response{ + StatusCode: http.StatusBadRequest, + Body: map[string]string{ + "error_code": "INVALID_PARAMETER_VALUE", + "message": "UpdateSchema Nothing to update.", + }, + } + } + var schemaUpdate catalog.SchemaInfo if err := json.Unmarshal(req.Body, &schemaUpdate); err != nil { @@ -106,3 +128,14 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { Body: existing, } } + +// hasFieldToUpdate reports whether an update payload carries at least one field the +// backend can act on. A key set to null does not count. +func hasFieldToUpdate(fields map[string]json.RawMessage) bool { + for _, value := range fields { + if string(value) != "null" { + return true + } + } + return false +} From 4fa9e75054e9ce91e47a94c5d276f1bde241fd51 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 21 Aug 2026 18:16:12 +0200 Subject: [PATCH 02/16] Send comment explicitly on UpdateSchema so a cleared comment is not an empty PATCH Every UpdateSchema field is omitempty, so a schema whose config declares no comment produced an empty PATCH body once the comment was set out of band. UC answers that with `400 / UpdateSchema Nothing to update` rather than a no-op, which failed the whole deploy with no way out from the CLI. Force-send comment so the payload always carries a field and clearing a comment set outside the bundle actually happens. Co-authored-by: Isaac --- .../schema-comment-nothing-to-update.md | 1 + .../drift/comment_out_of_band/output.txt | 36 +++++-------------- .../schemas/drift/comment_out_of_band/script | 4 +-- bundle/direct/dresources/schema.go | 6 ++++ libs/testserver/schemas.go | 7 ++++ 5 files changed, 25 insertions(+), 29 deletions(-) create mode 100644 .nextchanges/bundles/schema-comment-nothing-to-update.md diff --git a/.nextchanges/bundles/schema-comment-nothing-to-update.md b/.nextchanges/bundles/schema-comment-nothing-to-update.md new file mode 100644 index 00000000000..01dc55eca9f --- /dev/null +++ b/.nextchanges/bundles/schema-comment-nothing-to-update.md @@ -0,0 +1 @@ +Fixed `bundle deploy` failing with `UpdateSchema Nothing to update` on the direct engine when a schema's comment was set outside the bundle. diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt index be6797d19ec..f82449ce7a2 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt @@ -15,36 +15,21 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged } === Redeploy clears the comment ->>> errcode [CLI] bundle deploy +>>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... -Error: cannot update resources.schemas.schema1: updating id=main.test-schema-[UNIQUE_NAME]: UpdateSchema Nothing to update. (400 INVALID_PARAMETER_VALUE) - -Endpoint: PATCH [DATABRICKS_URL]/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] -HTTP Status: 400 Bad Request -API error_code: INVALID_PARAMETER_VALUE -API message: UpdateSchema Nothing to update. - +Updated schemas.schema1 Files: 0 uploaded, 0 deleted - -Exit code: 1 +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged === The comment is gone >>> [CLI] api get /api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] -"set outside the bundle" +"" === Redeploy is a no-op ->>> errcode [CLI] bundle deploy +>>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... -Error: cannot update resources.schemas.schema1: updating id=main.test-schema-[UNIQUE_NAME]: UpdateSchema Nothing to update. (400 INVALID_PARAMETER_VALUE) - -Endpoint: PATCH [DATABRICKS_URL]/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] -HTTP Status: 400 Bad Request -API error_code: INVALID_PARAMETER_VALUE -API message: UpdateSchema Nothing to update. - Files: 0 uploaded, 0 deleted - -Exit code: 1 +Resources: 0 created, 0 changed, 0 deleted, 1 unchanged >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: @@ -69,10 +54,7 @@ Destroy: 1 deleted { "method": "PATCH", "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", - "body": {} -} -{ - "method": "PATCH", - "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", - "body": {} + "body": { + "comment": "" + } } diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script index dac79d7d8d5..b5f23ec918e 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script @@ -23,10 +23,10 @@ title "The remote comment is drift, so the plan updates the schema" trace $CLI bundle plan --output json | jq '.plan[].changes.comment' title "Redeploy clears the comment" -trace errcode $CLI bundle deploy +trace $CLI bundle deploy title "The comment is gone" MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | jq '.comment' title "Redeploy is a no-op" -trace errcode $CLI bundle deploy +trace $CLI bundle deploy diff --git a/bundle/direct/dresources/schema.go b/bundle/direct/dresources/schema.go index 24c82ceb6a6..86b7f81762c 100644 --- a/bundle/direct/dresources/schema.go +++ b/bundle/direct/dresources/schema.go @@ -59,6 +59,12 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalo ForceSendFields: utils.FilterFields[catalog.UpdateSchema](config.ForceSendFields, "EnablePredictiveOptimization", "NewName", "Owner"), } + // UC answers a PATCH that carries no field with "Nothing to update" (400) instead of + // treating it as a no-op, and omitempty drops every field the config leaves unset. + // Always sending comment keeps the payload non-empty and makes clearing a comment + // that was set outside the bundle explicit. + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, "Comment") + response, err := r.client.Schemas.Update(ctx, updateRequest) if err != nil { return nil, err diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index f685239a62f..f48c3e1596e 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -119,6 +119,13 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { } } + // mergo skips zero values, so an explicit empty comment would leave the stored one in + // place. UC clears it, and comment is the only field its UpdateSchema can clear: an + // empty or null properties map is what it rejects as "nothing to update" above. + if _, ok := fields["comment"]; ok { + existing.Comment = schemaUpdate.Comment + } + existing.UpdatedAt = nowMilli() existing.UpdatedBy = s.CurrentUser().UserName From bc10f7409bd003f1f8d5cd40e5ac723b12856f1a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 21 Aug 2026 18:22:26 +0200 Subject: [PATCH 03/16] Reword the testserver comment on clearing a schema comment Co-authored-by: Isaac --- libs/testserver/schemas.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index f48c3e1596e..7de58a8a2bf 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -120,8 +120,8 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { } // mergo skips zero values, so an explicit empty comment would leave the stored one in - // place. UC clears it, and comment is the only field its UpdateSchema can clear: an - // empty or null properties map is what it rejects as "nothing to update" above. + // place. UC clears it instead. Comment is the only field UpdateSchema can clear at + // all: clearing properties needs an empty or null map, which the check above rejects. if _, ok := fields["comment"]; ok { existing.Comment = schemaUpdate.Comment } From 2d89680c95d747aa681ace3aaf7f8b86b44361b7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Sat, 22 Aug 2026 09:06:34 +0200 Subject: [PATCH 04/16] Send comment explicitly on UpdateCatalog and UpdateVolume too Catalogs and volumes fail exactly like schemas did: their update payloads carry only fields the config may leave unset, so clearing a comment that was set out of band produced an empty PATCH and `400 / Nothing to update`. Verified against a real workspace for both. Force-send comment in all four update paths (catalogs and volumes each have a rename variant), moving the shared reason into forceSendComment, and teach the fake workspace to reject an empty payload and honour an explicit empty comment the way UC does. Co-authored-by: Isaac --- .../schema-comment-nothing-to-update.md | 2 +- .../comment_out_of_band/databricks.yml.tmpl | 7 ++ .../drift/comment_out_of_band/out.test.toml | 3 + .../drift/comment_out_of_band/output.txt | 57 ++++++++++++++++ .../catalogs/drift/comment_out_of_band/script | 32 +++++++++ .../drift/comment_out_of_band/test.toml | 11 +++ .../comment_out_of_band/databricks.yml.tmpl | 13 ++++ .../drift/comment_out_of_band/out.test.toml | 3 + .../drift/comment_out_of_band/output.txt | 68 +++++++++++++++++++ .../volumes/drift/comment_out_of_band/script | 32 +++++++++ .../drift/comment_out_of_band/test.toml | 11 +++ bundle/direct/dresources/catalog.go | 4 +- bundle/direct/dresources/schema.go | 8 +-- bundle/direct/dresources/util.go | 11 +++ bundle/direct/dresources/volume.go | 4 +- libs/testserver/catalogs.go | 13 +++- libs/testserver/fake_workspace.go | 32 +++++++++ libs/testserver/schemas.go | 34 +--------- libs/testserver/volumes.go | 12 +++- 19 files changed, 311 insertions(+), 46 deletions(-) create mode 100644 acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml create mode 100644 acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt create mode 100644 acceptance/bundle/resources/catalogs/drift/comment_out_of_band/script create mode 100644 acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml create mode 100644 acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml create mode 100644 acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt create mode 100644 acceptance/bundle/resources/volumes/drift/comment_out_of_band/script create mode 100644 acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml diff --git a/.nextchanges/bundles/schema-comment-nothing-to-update.md b/.nextchanges/bundles/schema-comment-nothing-to-update.md index 01dc55eca9f..d7e77b70434 100644 --- a/.nextchanges/bundles/schema-comment-nothing-to-update.md +++ b/.nextchanges/bundles/schema-comment-nothing-to-update.md @@ -1 +1 @@ -Fixed `bundle deploy` failing with `UpdateSchema Nothing to update` on the direct engine when a schema's comment was set outside the bundle. +Fixed `bundle deploy` failing with `Nothing to update` on the direct engine when a schema's, catalog's or volume's comment was set outside the bundle. diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl new file mode 100644 index 00000000000..4c684900b5b --- /dev/null +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl @@ -0,0 +1,7 @@ +bundle: + name: test-bundle + +resources: + catalogs: + catalog1: + name: test-catalog-$UNIQUE_NAME diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml new file mode 100644 index 00000000000..8c52d40aa2d --- /dev/null +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt new file mode 100644 index 00000000000..833502b60af --- /dev/null +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt @@ -0,0 +1,57 @@ + +=== Deploy a catalog that does not declare a comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Created catalogs.catalog1 +Files: 0 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 0 unchanged + +=== Set the comment out of band, the way Catalog Explorer does +=== The remote comment is drift, so the plan updates the catalog +>>> [CLI] bundle plan --output json +{ + "action": "update", + "remote": "set outside the bundle" +} + +=== Redeploy clears the comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Updated catalogs.catalog1 +Files: 0 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged + +=== The comment is gone +>>> [CLI] api get /api/2.1/unity-catalog/catalogs/test-catalog-[UNIQUE_NAME] +"" + +=== Redeploy is a no-op +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Files: 0 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 1 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.catalogs.catalog1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Destroy: 1 deleted + +=== The UpdateCatalog payloads the deploys sent +>>> print_requests.py --method PATCH //unity-catalog/catalogs +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/catalogs/test-catalog-[UNIQUE_NAME]", + "body": { + "comment": "set outside the bundle" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/catalogs/test-catalog-[UNIQUE_NAME]", + "body": { + "comment": "" + } +} diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/script b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/script new file mode 100644 index 00000000000..6f1a938177d --- /dev/null +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/script @@ -0,0 +1,32 @@ +echo "*" > .gitignore +envsubst < databricks.yml.tmpl > databricks.yml + +CATALOG="test-catalog-${UNIQUE_NAME}" + +# print_requests.py consumes out.requests.txt, so it has to run after the last +# request the test makes, which is the destroy below. +cleanup() { + trace $CLI bundle destroy --auto-approve + + title "The UpdateCatalog payloads the deploys sent" + trace print_requests.py --method PATCH //unity-catalog/catalogs +} +trap cleanup EXIT + +title "Deploy a catalog that does not declare a comment" +trace $CLI bundle deploy + +title "Set the comment out of band, the way Catalog Explorer does" +MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/catalogs/$CATALOG" --json '{"comment":"set outside the bundle"}' > /dev/null + +title "The remote comment is drift, so the plan updates the catalog" +trace $CLI bundle plan --output json | jq '.plan[].changes.comment' + +title "Redeploy clears the comment" +trace $CLI bundle deploy + +title "The comment is gone" +MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/catalogs/$CATALOG" | jq '.comment' + +title "Redeploy is a no-op" +trace $CLI bundle deploy diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml new file mode 100644 index 00000000000..ae0b1a870d3 --- /dev/null +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml @@ -0,0 +1,11 @@ +Cloud = true +RequiresUnityCatalog = true + +# UpdateCatalog payload construction is direct-engine-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", +] diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl new file mode 100644 index 00000000000..dd5d0340df0 --- /dev/null +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl @@ -0,0 +1,13 @@ +bundle: + name: test-bundle + +resources: + schemas: + schema1: + catalog_name: main + name: test-schema-$UNIQUE_NAME + volumes: + volume1: + catalog_name: main + schema_name: ${resources.schemas.schema1.name} + name: test-volume-$UNIQUE_NAME diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml new file mode 100644 index 00000000000..8c52d40aa2d --- /dev/null +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt new file mode 100644 index 00000000000..6479c07eec7 --- /dev/null +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt @@ -0,0 +1,68 @@ + +=== Deploy a volume that does not declare a comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Created schemas.schema1 +Created volumes.volume1 +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== Set the comment out of band, the way Catalog Explorer does +=== The remote comment is drift, so the plan updates the volume +>>> [CLI] bundle plan --output json +{ + "action": "update", + "remote": "set outside the bundle" +} + +=== Redeploy clears the comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Updated volumes.volume1 +Files: 0 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 1 unchanged + +=== The comment is gone +>>> [CLI] api get /api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].test-volume-[UNIQUE_NAME] +"" + +=== Redeploy is a no-op +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Files: 0 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 2 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.schema1 + delete resources.volumes.volume1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +This action will result in the deletion of the following volumes. +For managed volumes, the files stored in the volume are also deleted from your +cloud tenant within 30 days. For external volumes, the metadata about the volume +is removed from the catalog, but the underlying files are not deleted: + delete resources.volumes.volume1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Destroy: 2 deleted + +=== The UpdateVolume payloads the deploys sent +>>> print_requests.py --method PATCH //unity-catalog/volumes +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].test-volume-[UNIQUE_NAME]", + "body": { + "comment": "set outside the bundle" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].test-volume-[UNIQUE_NAME]", + "body": { + "comment": "" + } +} diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/script b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/script new file mode 100644 index 00000000000..c176403ba00 --- /dev/null +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/script @@ -0,0 +1,32 @@ +echo "*" > .gitignore +envsubst < databricks.yml.tmpl > databricks.yml + +VOLUME="main.test-schema-${UNIQUE_NAME}.test-volume-${UNIQUE_NAME}" + +# print_requests.py consumes out.requests.txt, so it has to run after the last +# request the test makes, which is the destroy below. +cleanup() { + trace $CLI bundle destroy --auto-approve + + title "The UpdateVolume payloads the deploys sent" + trace print_requests.py --method PATCH //unity-catalog/volumes +} +trap cleanup EXIT + +title "Deploy a volume that does not declare a comment" +trace $CLI bundle deploy + +title "Set the comment out of band, the way Catalog Explorer does" +MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/volumes/$VOLUME" --json '{"comment":"set outside the bundle"}' > /dev/null + +title "The remote comment is drift, so the plan updates the volume" +trace $CLI bundle plan --output json | jq '.plan["resources.volumes.volume1"].changes.comment' + +title "Redeploy clears the comment" +trace $CLI bundle deploy + +title "The comment is gone" +MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/volumes/$VOLUME" | jq '.comment' + +title "Redeploy is a no-op" +trace $CLI bundle deploy diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml new file mode 100644 index 00000000000..bec056028b6 --- /dev/null +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml @@ -0,0 +1,11 @@ +Cloud = true +RequiresUnityCatalog = true + +# UpdateVolume payload construction is direct-engine-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", +] diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 645fb933cd1..aa2cf05994b 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -62,7 +62,7 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), + ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner")), } response, err := r.client.Catalogs.Update(ctx, updateRequest) @@ -86,7 +86,7 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), + ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner")), } if config.Name != id { diff --git a/bundle/direct/dresources/schema.go b/bundle/direct/dresources/schema.go index 86b7f81762c..ea406e2684e 100644 --- a/bundle/direct/dresources/schema.go +++ b/bundle/direct/dresources/schema.go @@ -56,15 +56,9 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // We recreate schemas on name change intentionally. Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateSchema](config.ForceSendFields, "EnablePredictiveOptimization", "NewName", "Owner"), + ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateSchema](config.ForceSendFields, "EnablePredictiveOptimization", "NewName", "Owner")), } - // UC answers a PATCH that carries no field with "Nothing to update" (400) instead of - // treating it as a no-op, and omitempty drops every field the config leaves unset. - // Always sending comment keeps the payload non-empty and makes clearing a comment - // that was set outside the bundle explicit. - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, "Comment") - response, err := r.client.Schemas.Update(ctx, updateRequest) if err != nil { return nil, err diff --git a/bundle/direct/dresources/util.go b/bundle/direct/dresources/util.go index fc68f3ea37c..51db2a5dcdf 100644 --- a/bundle/direct/dresources/util.go +++ b/bundle/direct/dresources/util.go @@ -65,3 +65,14 @@ func collectLeafUpdatePathsWithPrefix(changes Changes, prefix string) []string { slices.Sort(paths) return paths } + +// forceSendComment adds Comment to a ForceSendFields list, so a comment the config no +// longer sets serializes as "" instead of being dropped by omitempty. +// +// The UC update APIs answer a PATCH that carries no field with "Nothing to update" (400) +// rather than treating it as a no-op. A resource whose only updatable field is its comment +// would otherwise become undeployable the moment someone sets a description on it outside +// the bundle. Sending the empty value also makes clearing such a comment actually happen. +func forceSendComment(forceSendFields []string) []string { + return append(forceSendFields, "Comment") +} diff --git a/bundle/direct/dresources/volume.go b/bundle/direct/dresources/volume.go index 9d33f31ec71..7d657a940d0 100644 --- a/bundle/direct/dresources/volume.go +++ b/bundle/direct/dresources/volume.go @@ -55,7 +55,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // Not supported by Update(). Needs DoUpdateWithID() Owner: "", // Not supported by DABs - ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "NewName", "Owner"), + ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "NewName", "Owner")), } nameFromID, err := getNameFromID(id) @@ -87,7 +87,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * NewName: "", // Initialized below if needed Owner: "", // Not supported by DABs - ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "Owner"), + ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "Owner")), } items := strings.Split(id, ".") diff --git a/libs/testserver/catalogs.go b/libs/testserver/catalogs.go index 1d0bc065681..4c1c864b1a4 100644 --- a/libs/testserver/catalogs.go +++ b/libs/testserver/catalogs.go @@ -88,6 +88,11 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { } } + fields, errResponse := parseUCUpdate(req.Body, "UpdateCatalog") + if errResponse != nil { + return *errResponse + } + var updateRequest catalog.UpdateCatalog if err := json.Unmarshal(req.Body, &updateRequest); err != nil { return Response{ @@ -96,9 +101,13 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { } } - // Update only the fields that can be updated - if updateRequest.Comment != "" { + // Update only the fields that can be updated. Comment is keyed off presence rather + // than a non-empty value so an explicit empty comment clears it, the way UC does. + if _, ok := fields["comment"]; ok { existing.Comment = updateRequest.Comment + // UC echoes a comment once it has been set, including when it was set to "", + // so keep it in the response rather than letting omitempty drop it. + existing.ForceSendFields = append(existing.ForceSendFields, "Comment") } if updateRequest.CustomMaxRetentionHours != 0 { existing.CustomMaxRetentionHours = updateRequest.CustomMaxRetentionHours diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 25178a9e75f..6c4c940292f 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "encoding/json" "fmt" + "net/http" "os" "path" "path/filepath" @@ -255,6 +256,37 @@ func (s *FakeWorkspace) LockUnlock() func() { return func() { s.mu.Unlock() } } +// parseUCUpdate decodes a Unity Catalog update payload into its raw fields. It returns a +// rejection response when the body carries no field to act on: UC answers such a PATCH with +// " Nothing to update." (400) rather than treating it as a no-op. A key set to +// null does not count. Verified against a real workspace for schemas, volumes and catalogs: +// {} and {"comment": null} are rejected, while {"comment": ""} and +// {"custom_max_retention_hours": 0} are accepted. +func parseUCUpdate(body []byte, operation string) (map[string]json.RawMessage, *Response) { + var fields map[string]json.RawMessage + + if err := json.Unmarshal(body, &fields); err != nil { + return nil, &Response{ + Body: fmt.Sprintf("internal error: %s", err), + StatusCode: http.StatusInternalServerError, + } + } + + for _, value := range fields { + if string(value) != "null" { + return fields, nil + } + } + + return nil, &Response{ + StatusCode: http.StatusBadRequest, + Body: map[string]string{ + "error_code": "INVALID_PARAMETER_VALUE", + "message": operation + " Nothing to update.", + }, + } +} + // Generic functions to handle map operations func MapGet[T any](w *FakeWorkspace, collection map[string]T, key string) Response { defer w.LockUnlock()() diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index 7de58a8a2bf..72fd4564278 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -80,26 +80,9 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { } } - var fields map[string]json.RawMessage - - if err := json.Unmarshal(req.Body, &fields); err != nil { - return Response{ - Body: fmt.Sprintf("internal error: %s", err), - StatusCode: http.StatusInternalServerError, - } - } - - // UC rejects a PATCH that carries no field to update instead of treating it as a - // no-op. Verified against a real workspace: {} and {"comment": null} are rejected, - // while {"comment": ""} and {"custom_max_retention_hours": 0} are accepted. - if !hasFieldToUpdate(fields) { - return Response{ - StatusCode: http.StatusBadRequest, - Body: map[string]string{ - "error_code": "INVALID_PARAMETER_VALUE", - "message": "UpdateSchema Nothing to update.", - }, - } + fields, errResponse := parseUCUpdate(req.Body, "UpdateSchema") + if errResponse != nil { + return *errResponse } var schemaUpdate catalog.SchemaInfo @@ -135,14 +118,3 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { Body: existing, } } - -// hasFieldToUpdate reports whether an update payload carries at least one field the -// backend can act on. A key set to null does not count. -func hasFieldToUpdate(fields map[string]json.RawMessage) bool { - for _, value := range fields { - if string(value) != "null" { - return true - } - } - return false -} diff --git a/libs/testserver/volumes.go b/libs/testserver/volumes.go index 88eae7ac021..5a6923b0e89 100644 --- a/libs/testserver/volumes.go +++ b/libs/testserver/volumes.go @@ -76,6 +76,11 @@ func (s *FakeWorkspace) VolumesUpdate(req Request, fullname string) Response { } } + fields, errResponse := parseUCUpdate(req.Body, "UpdateVolume") + if errResponse != nil { + return *errResponse + } + var request catalog.UpdateVolumeRequestContent if err := json.Unmarshal(req.Body, &request); err != nil { @@ -85,8 +90,13 @@ func (s *FakeWorkspace) VolumesUpdate(req Request, fullname string) Response { } } - if request.Comment != "" { + // Keyed off presence rather than a non-empty value so an explicit empty comment + // clears the stored one, the way UC does. + if _, ok := fields["comment"]; ok { existing.Comment = request.Comment + // UC echoes a comment once it has been set, including when it was set to "", + // so keep it in the response rather than letting omitempty drop it. + existing.ForceSendFields = append(existing.ForceSendFields, "Comment") } if request.Owner != "" { From 65ca3ca9931c10257011a41d4921e1caea65197f Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Sat, 22 Aug 2026 11:42:46 +0200 Subject: [PATCH 05/16] Make the comment drift fixtures use a unique bundle name The bundle name is the workspace state path, and cloud tests share one real workspace, so the hardcoded "test-bundle" made these three fight over the same deploy.lock as every other test using that name. They passed run alone and failed under parallelism: the integration run reported success while retrying them on nearly every environment. Co-authored-by: Isaac --- .../drift/comment_out_of_band/databricks.yml.tmpl | 2 +- .../catalogs/drift/comment_out_of_band/output.txt | 8 ++++---- .../schemas/drift/comment_out_of_band/databricks.yml.tmpl | 2 +- .../schemas/drift/comment_out_of_band/output.txt | 8 ++++---- .../volumes/drift/comment_out_of_band/databricks.yml.tmpl | 2 +- .../volumes/drift/comment_out_of_band/output.txt | 8 ++++---- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl index 4c684900b5b..b7fa6d6d952 100644 --- a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl @@ -1,5 +1,5 @@ bundle: - name: test-bundle + name: test-bundle-$UNIQUE_NAME resources: catalogs: diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt index 833502b60af..6ff82f588e6 100644 --- a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt @@ -1,7 +1,7 @@ === Deploy a catalog that does not declare a comment >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Created catalogs.catalog1 Files: 0 uploaded, 0 deleted Resources: 1 created, 0 changed, 0 deleted, 0 unchanged @@ -16,7 +16,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged === Redeploy clears the comment >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Updated catalogs.catalog1 Files: 0 uploaded, 0 deleted Resources: 0 created, 1 changed, 0 deleted, 0 unchanged @@ -27,7 +27,7 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged === Redeploy is a no-op >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 1 unchanged @@ -35,7 +35,7 @@ Resources: 0 created, 0 changed, 0 deleted, 1 unchanged The following resources will be deleted: delete resources.catalogs.catalog1 -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default Destroy: 1 deleted diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl index e3283c6b3bf..50720b62560 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl @@ -1,5 +1,5 @@ bundle: - name: test-bundle + name: test-bundle-$UNIQUE_NAME resources: schemas: diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt index f82449ce7a2..d45016243ea 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt @@ -1,7 +1,7 @@ === Deploy a schema that does not declare a comment >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Created schemas.schema1 Files: 0 uploaded, 0 deleted Resources: 1 created, 0 changed, 0 deleted, 0 unchanged @@ -16,7 +16,7 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged === Redeploy clears the comment >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Updated schemas.schema1 Files: 0 uploaded, 0 deleted Resources: 0 created, 1 changed, 0 deleted, 0 unchanged @@ -27,7 +27,7 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged === Redeploy is a no-op >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 1 unchanged @@ -38,7 +38,7 @@ The following resources will be deleted: This action will result in the deletion of the following UC schemas. Any underlying data may be lost: delete resources.schemas.schema1 -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default Destroy: 1 deleted diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl index dd5d0340df0..084b752f4a8 100644 --- a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl @@ -1,5 +1,5 @@ bundle: - name: test-bundle + name: test-bundle-$UNIQUE_NAME resources: schemas: diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt index 6479c07eec7..c1e1f119fa1 100644 --- a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt @@ -1,7 +1,7 @@ === Deploy a volume that does not declare a comment >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Created schemas.schema1 Created volumes.volume1 Files: 0 uploaded, 0 deleted @@ -17,7 +17,7 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged === Redeploy clears the comment >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Updated volumes.volume1 Files: 0 uploaded, 0 deleted Resources: 0 created, 1 changed, 0 deleted, 1 unchanged @@ -28,7 +28,7 @@ Resources: 0 created, 1 changed, 0 deleted, 1 unchanged === Redeploy is a no-op >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 2 unchanged @@ -46,7 +46,7 @@ cloud tenant within 30 days. For external volumes, the metadata about the volume is removed from the catalog, but the underlying files are not deleted: delete resources.volumes.volume1 -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default Destroy: 2 deleted From ae34e6adfed3140bdd87104aba19a2992aea1c11 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 24 Aug 2026 11:31:46 +0200 Subject: [PATCH 06/16] Force-send the fields the plan reports as cleared, not comment unconditionally The previous commits always force-sent comment, so a deploy whose only drift was another field still put `"comment": ""` on the wire, and a field the plan had classified as skip would have been cleared behind the plan's back. Any other omitempty field the config stops setting was still dropped, so it never converged. Derive the force-send list from the plan instead, matched against the request type's own JSON names. Each PATCH now carries exactly what the plan says is changing, and roughly 40 omitempty fields across the UC resources converge instead of only comment. forceSendClearedFields documents the general shape, including why full-replacement APIs (jobs, pipelines, model serving) need none of it. Also drop the special-casing in the fake workspace: applyUpdatedFields applies whatever the payload names, including zero values, which is what a partial-update API does. That incidentally stops mergo from clobbering the stored ForceSendFields, so browse_only survives an update the way UC returns it. Co-authored-by: Isaac --- .../schema-comment-nothing-to-update.md | 2 +- .../drift/comment_out_of_band/output.txt | 28 +++++++++++ .../schemas/drift/comment_out_of_band/script | 9 ++++ bundle/direct/dresources/all_test.go | 2 + bundle/direct/dresources/catalog.go | 11 +++-- bundle/direct/dresources/schema.go | 6 ++- bundle/direct/dresources/schema_test.go | 3 ++ bundle/direct/dresources/util.go | 48 +++++++++++++++---- bundle/direct/dresources/volume.go | 9 ++-- libs/testserver/catalogs.go | 24 +--------- libs/testserver/fake_workspace.go | 34 +++++++++++++ libs/testserver/schemas.go | 16 +------ libs/testserver/volumes.go | 13 +---- 13 files changed, 138 insertions(+), 67 deletions(-) diff --git a/.nextchanges/bundles/schema-comment-nothing-to-update.md b/.nextchanges/bundles/schema-comment-nothing-to-update.md index d7e77b70434..7267769243a 100644 --- a/.nextchanges/bundles/schema-comment-nothing-to-update.md +++ b/.nextchanges/bundles/schema-comment-nothing-to-update.md @@ -1 +1 @@ -Fixed `bundle deploy` failing with `Nothing to update` on the direct engine when a schema's, catalog's or volume's comment was set outside the bundle. +Fixed `bundle deploy` on the direct engine failing with `Nothing to update`, or silently not converging, when a schema, catalog or volume field was set outside the bundle. The update now sends the fields the plan reports as cleared instead of dropping them. diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt index d45016243ea..bffdb9c8d36 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt @@ -31,6 +31,20 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQU Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 1 unchanged +=== The same holds for a non-string field set out of band +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Updated schemas.schema1 +Files: 0 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged + +>>> [CLI] api get /api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] +0 + +=== And it converges +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: delete resources.schemas.schema1 @@ -58,3 +72,17 @@ Destroy: 1 deleted "comment": "" } } +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", + "body": { + "custom_max_retention_hours": 240 + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", + "body": { + "custom_max_retention_hours": 0 + } +} diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script index b5f23ec918e..3c2514950e0 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script @@ -30,3 +30,12 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | title "Redeploy is a no-op" trace $CLI bundle deploy + +# 240h (10 days) is inside UC's valid 7-30 day window; smaller values are rejected. +title "The same holds for a non-string field set out of band" +MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/schemas/$SCHEMA" --json '{"custom_max_retention_hours":240}' > /dev/null +trace $CLI bundle deploy +MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | jq '.custom_max_retention_hours' + +title "And it converges" +trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged" diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 0c0d3d05ada..36eaa3e27df 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -63,6 +63,7 @@ var testConfig map[string]any = map[string]any{ CreateSchema: catalog.CreateSchema{ CatalogName: "main", Name: "myschema", + Comment: "Test schema", }, }, @@ -71,6 +72,7 @@ var testConfig map[string]any = map[string]any{ CatalogName: "main", SchemaName: "myschema", Name: "myvolume", + Comment: "Test volume", }, }, diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index aa2cf05994b..ec3691d213d 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -50,7 +50,7 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa } // DoUpdate updates the catalog in place and returns remote state. -func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, _ *PlanEntry) (*catalog.CatalogInfo, error) { +func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, entry *PlanEntry) (*catalog.CatalogInfo, error) { updateRequest := catalog.UpdateCatalog{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -62,9 +62,11 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner")), + ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), } + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { return nil, err @@ -86,13 +88,16 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner")), + ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), } if config.Name != id { updateRequest.NewName = config.Name } + // No forceSendClearedFields here: the rename always carries new_name, so the payload is + // never empty, and DoUpdateWithID has no PlanEntry to consult. A field cleared in the + // same deploy as a rename is picked up by the DoUpdate on the next one. response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { return "", nil, err diff --git a/bundle/direct/dresources/schema.go b/bundle/direct/dresources/schema.go index ea406e2684e..6f77d19e7e2 100644 --- a/bundle/direct/dresources/schema.go +++ b/bundle/direct/dresources/schema.go @@ -47,7 +47,7 @@ func (r *ResourceSchema) DoCreate(ctx context.Context, config *catalog.CreateSch } // DoUpdate updates the schema in place and returns remote state. -func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalog.CreateSchema, _ *PlanEntry) (*catalog.SchemaInfo, error) { +func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalog.CreateSchema, entry *PlanEntry) (*catalog.SchemaInfo, error) { updateRequest := catalog.UpdateSchema{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -56,9 +56,11 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // We recreate schemas on name change intentionally. Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateSchema](config.ForceSendFields, "EnablePredictiveOptimization", "NewName", "Owner")), + ForceSendFields: utils.FilterFields[catalog.UpdateSchema](config.ForceSendFields, "EnablePredictiveOptimization", "NewName", "Owner"), } + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + response, err := r.client.Schemas.Update(ctx, updateRequest) if err != nil { return nil, err diff --git a/bundle/direct/dresources/schema_test.go b/bundle/direct/dresources/schema_test.go index d013610e052..584b50816ab 100644 --- a/bundle/direct/dresources/schema_test.go +++ b/bundle/direct/dresources/schema_test.go @@ -49,7 +49,10 @@ func TestResourceSchema_DoUpdate_WithUnsupportedForceSendFields(t *testing.T) { resultJSON, err := json.Marshal(result) require.NoError(t, err) + // browse_only is force-sent from create onwards, matching UC, which returns it on + // every read including after an update. expected := `{ + "browse_only": false, "catalog_name": "main", "catalog_type": "MANAGED_CATALOG", "created_at": 0, diff --git a/bundle/direct/dresources/util.go b/bundle/direct/dresources/util.go index 51db2a5dcdf..9a10ecce691 100644 --- a/bundle/direct/dresources/util.go +++ b/bundle/direct/dresources/util.go @@ -2,10 +2,13 @@ package dresources import ( "errors" + "reflect" "slices" "strings" "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/libs/structs/structpath" + "github.com/databricks/cli/libs/structs/structtag" "github.com/databricks/databricks-sdk-go/retries" ) @@ -66,13 +69,42 @@ func collectLeafUpdatePathsWithPrefix(changes Changes, prefix string) []string { return paths } -// forceSendComment adds Comment to a ForceSendFields list, so a comment the config no -// longer sets serializes as "" instead of being dropped by omitempty. +// forceSendClearedFields returns ForceSendFields entries for the request fields the plan +// is clearing: the plan reports a change, but the value is zero, so omitempty would drop +// the field from the payload entirely. // -// The UC update APIs answer a PATCH that carries no field with "Nothing to update" (400) -// rather than treating it as a no-op. A resource whose only updatable field is its comment -// would otherwise become undeployable the moment someone sets a description on it outside -// the bundle. Sending the empty value also makes clearing such a comment actually happen. -func forceSendComment(forceSendFields []string) []string { - return append(forceSendFields, "Comment") +// A partial-update API reads an absent field as "leave unchanged", so dropping it +// silently discards the clear and the next plan reports the same drift forever. Unity +// Catalog goes further and rejects a payload carrying no field at all with +// " Nothing to update." (400), which made a schema, catalog or volume +// undeployable as soon as someone set a description on it outside the bundle (#6340). +// +// Every omitempty field assigned from config into a partial-update request shares the +// problem -- roughly 40 of them across the UC resources, not just comment -- so this is +// driven off the plan instead of naming individual fields. Resources whose update +// replaces the whole object (jobs, pipelines, model serving) need none of it: there an +// absent field already means cleared, and they converge on their own. +// +// Matching is by the request type's own JSON names, so a field the request does not +// expose, or names differently (e.g. new_name), is never force-sent. Skipped changes are +// excluded by HasChange, so a field the plan suppressed -- via backend_defaults or +// ignore_remote_changes -- is left alone rather than being cleared behind the plan's back. +func forceSendClearedFields(request any, changes Changes) []string { + value := reflect.ValueOf(request).Elem() + valueType := value.Type() + + var result []string + for i := range valueType.NumField() { + field := valueType.Field(i) + name := structtag.JSONTag(field.Tag.Get("json")).Name() + if name == "" || name == "-" || !value.Field(i).IsZero() { + continue + } + if changes.HasChange(structpath.NewDotString(nil, name)) { + result = append(result, field.Name) + } + } + + slices.Sort(result) + return result } diff --git a/bundle/direct/dresources/volume.go b/bundle/direct/dresources/volume.go index 7d657a940d0..09cfedfe74d 100644 --- a/bundle/direct/dresources/volume.go +++ b/bundle/direct/dresources/volume.go @@ -48,14 +48,14 @@ func (r *ResourceVolume) DoCreate(ctx context.Context, config *catalog.CreateVol return response.FullName, response, nil } -func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, _ *PlanEntry) (*catalog.VolumeInfo, error) { +func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, entry *PlanEntry) (*catalog.VolumeInfo, error) { updateRequest := catalog.UpdateVolumeRequestContent{ Comment: config.Comment, Name: id, NewName: "", // Not supported by Update(). Needs DoUpdateWithID() Owner: "", // Not supported by DABs - ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "NewName", "Owner")), + ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "NewName", "Owner"), } nameFromID, err := getNameFromID(id) @@ -67,6 +67,8 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo return nil, fmt.Errorf("internal error: unexpected change of name from %#v to %#v", nameFromID, config.Name) } + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil { return nil, err @@ -87,7 +89,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * NewName: "", // Initialized below if needed Owner: "", // Not supported by DABs - ForceSendFields: forceSendComment(utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "Owner")), + ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "Owner"), } items := strings.Split(id, ".") @@ -100,6 +102,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * updateRequest.NewName = config.Name } + // See ResourceCatalog.DoUpdateWithID on why the rename path does not force-send. response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil || response == nil { return "", nil, err diff --git a/libs/testserver/catalogs.go b/libs/testserver/catalogs.go index 4c1c864b1a4..347b97082a5 100644 --- a/libs/testserver/catalogs.go +++ b/libs/testserver/catalogs.go @@ -101,29 +101,7 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { } } - // Update only the fields that can be updated. Comment is keyed off presence rather - // than a non-empty value so an explicit empty comment clears it, the way UC does. - if _, ok := fields["comment"]; ok { - existing.Comment = updateRequest.Comment - // UC echoes a comment once it has been set, including when it was set to "", - // so keep it in the response rather than letting omitempty drop it. - existing.ForceSendFields = append(existing.ForceSendFields, "Comment") - } - if updateRequest.CustomMaxRetentionHours != 0 { - existing.CustomMaxRetentionHours = updateRequest.CustomMaxRetentionHours - } - if updateRequest.ManagedEncryptionSettings != nil { - existing.ManagedEncryptionSettings = updateRequest.ManagedEncryptionSettings - } - if updateRequest.Options != nil { - existing.Options = updateRequest.Options - } - if updateRequest.Properties != nil { - existing.Properties = updateRequest.Properties - } - if updateRequest.Owner != "" { - existing.Owner = updateRequest.Owner - } + applyUpdatedFields(&existing, updateRequest, fields) if updateRequest.NewName != "" { existing.Name = updateRequest.NewName existing.FullName = updateRequest.NewName diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 6c4c940292f..b568603dc73 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -9,6 +9,7 @@ import ( "os" "path" "path/filepath" + "reflect" "slices" "strconv" "strings" @@ -21,6 +22,7 @@ import ( "github.com/databricks/databricks-sdk-go/service/postgres" "github.com/google/uuid" + "github.com/databricks/cli/libs/structs/structtag" "github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/files" @@ -287,6 +289,38 @@ func parseUCUpdate(body []byte, operation string) (map[string]json.RawMessage, * } } +// applyUpdatedFields copies every field the update payload names from update onto +// existing, matched by JSON name, and marks it force-send so a zero value survives the +// response encoding (the stored *Info types are all omitempty). +// +// Fields the payload omits are left untouched: a partial-update API changes only what the +// caller names, and modelling that is the whole point of these fakes. Fields the payload +// names but the stored type lacks (new_name, force) are skipped for the caller to handle. +// existing must be a pointer; update is passed by value. +func applyUpdatedFields(existing, update any, fields map[string]json.RawMessage) { + dst := reflect.ValueOf(existing).Elem() + src := reflect.ValueOf(update) + + for i := range src.Type().NumField() { + name := structtag.JSONTag(src.Type().Field(i).Tag.Get("json")).Name() + if name == "" || name == "-" { + continue + } + if _, ok := fields[name]; !ok { + continue + } + dstField := dst.FieldByName(src.Type().Field(i).Name) + if !dstField.IsValid() || !dstField.CanSet() || dstField.Type() != src.Field(i).Type() { + continue + } + dstField.Set(src.Field(i)) + forceSend := dst.FieldByName("ForceSendFields") + if forceSend.IsValid() && forceSend.CanSet() { + forceSend.Set(reflect.Append(forceSend, reflect.ValueOf(src.Type().Field(i).Name))) + } + } +} + // Generic functions to handle map operations func MapGet[T any](w *FakeWorkspace, collection map[string]T, key string) Response { defer w.LockUnlock()() diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index 72fd4564278..6880a55cb8f 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -6,7 +6,6 @@ import ( "net/http" "strings" - "dario.cat/mergo" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -94,20 +93,7 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { } } - err := mergo.Merge(&existing, schemaUpdate, mergo.WithOverride) - if err != nil { - return Response{ - Body: fmt.Sprintf("mergo error: %s", err), - StatusCode: http.StatusInternalServerError, - } - } - - // mergo skips zero values, so an explicit empty comment would leave the stored one in - // place. UC clears it instead. Comment is the only field UpdateSchema can clear at - // all: clearing properties needs an empty or null map, which the check above rejects. - if _, ok := fields["comment"]; ok { - existing.Comment = schemaUpdate.Comment - } + applyUpdatedFields(&existing, schemaUpdate, fields) existing.UpdatedAt = nowMilli() existing.UpdatedBy = s.CurrentUser().UserName diff --git a/libs/testserver/volumes.go b/libs/testserver/volumes.go index 5a6923b0e89..91f29a94e4e 100644 --- a/libs/testserver/volumes.go +++ b/libs/testserver/volumes.go @@ -90,18 +90,7 @@ func (s *FakeWorkspace) VolumesUpdate(req Request, fullname string) Response { } } - // Keyed off presence rather than a non-empty value so an explicit empty comment - // clears the stored one, the way UC does. - if _, ok := fields["comment"]; ok { - existing.Comment = request.Comment - // UC echoes a comment once it has been set, including when it was set to "", - // so keep it in the response rather than letting omitempty drop it. - existing.ForceSendFields = append(existing.ForceSendFields, "Comment") - } - - if request.Owner != "" { - existing.Owner = request.Owner - } + applyUpdatedFields(&existing, request, fields) if request.NewName != "" { delete(s.Volumes, fullname) From c481f6564e8d55c100ffdf621949dfb12a6e81af Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 24 Aug 2026 17:40:21 +0200 Subject: [PATCH 07/16] Reproduce the dropped comment clear on registered models and external locations Removing a comment from the configuration reports a successful update and then never converges: the field is omitempty, so it leaves the payload entirely, and a partial-update API reads its absence as "leave unchanged". The comment stays, and every later plan reports the same pending change. Unlike the schema case this needs no out-of-band edit, and there is no error to notice -- the deploy says "1 changed" while changing nothing. The external locations golden also gains four synced files, since the new test lives inside that test's bundle root. Co-authored-by: Isaac --- .../drift/comment_cleared/databricks.yml | 10 ++++ .../drift/comment_cleared/out.test.toml | 3 ++ .../drift/comment_cleared/output.txt | 43 ++++++++++++++++ .../drift/comment_cleared/script | 24 +++++++++ .../drift/comment_cleared/test.toml | 16 ++++++ .../resources/external_locations/output.txt | 2 +- .../drift/comment_cleared/databricks.yml.tmpl | 14 ++++++ .../drift/comment_cleared/out.test.toml | 3 ++ .../drift/comment_cleared/output.txt | 49 +++++++++++++++++++ .../drift/comment_cleared/script | 27 ++++++++++ .../drift/comment_cleared/test.toml | 12 +++++ 11 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 acceptance/bundle/resources/external_locations/drift/comment_cleared/databricks.yml create mode 100644 acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml create mode 100644 acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt create mode 100644 acceptance/bundle/resources/external_locations/drift/comment_cleared/script create mode 100644 acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml create mode 100644 acceptance/bundle/resources/registered_models/drift/comment_cleared/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml create mode 100644 acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt create mode 100644 acceptance/bundle/resources/registered_models/drift/comment_cleared/script create mode 100644 acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/databricks.yml b/acceptance/bundle/resources/external_locations/drift/comment_cleared/databricks.yml new file mode 100644 index 00000000000..f7cd8e3811d --- /dev/null +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/databricks.yml @@ -0,0 +1,10 @@ +bundle: + name: test-bundle + +resources: + external_locations: + location1: + name: myloc + url: s3://test-bucket/path + credential_name: test_storage_credential + comment: declared in the bundle diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml b/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml new file mode 100644 index 00000000000..687990d50ea --- /dev/null +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml @@ -0,0 +1,3 @@ +Cloud = false +RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt b/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt new file mode 100644 index 00000000000..e56e7686322 --- /dev/null +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt @@ -0,0 +1,43 @@ + +=== Deploy with a comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Created external_locations.location1 +Files: 0 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 0 unchanged + +=== Remove the comment from the configuration and redeploy +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Updated external_locations.location1 +Files: 0 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged + +=== The comment the bundle no longer declares +>>> [CLI] api get /api/2.1/unity-catalog/external-locations/myloc +"declared in the bundle" + +=== Redeploy +>>> [CLI] bundle plan +update external_locations.location1 + +Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.external_locations.location1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Destroy: 1 deleted + +=== The UpdateExternalLocation payloads the deploys sent +>>> print_requests.py --method PATCH //external-locations +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/external-locations/myloc", + "body": { + "credential_name": "test_storage_credential", + "url": "s3://test-bucket/path" + } +} diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/script b/acceptance/bundle/resources/external_locations/drift/comment_cleared/script new file mode 100644 index 00000000000..0a5871d39f1 --- /dev/null +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/script @@ -0,0 +1,24 @@ +echo "*" > .gitignore +cp databricks.yml databricks.yml.orig + +# print_requests.py consumes out.requests.txt, so it runs after the last request. +cleanup() { + trace $CLI bundle destroy --auto-approve + + title "The UpdateExternalLocation payloads the deploys sent" + trace print_requests.py --method PATCH //external-locations +} +trap cleanup EXIT + +title "Deploy with a comment" +trace $CLI bundle deploy + +title "Remove the comment from the configuration and redeploy" +grep -v 'comment:' databricks.yml.orig > databricks.yml +trace $CLI bundle deploy + +title "The comment the bundle no longer declares" +MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/external-locations/myloc" | jq '.comment' + +title "Redeploy" +trace $CLI bundle plan diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml b/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml new file mode 100644 index 00000000000..f7a6c13375b --- /dev/null +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml @@ -0,0 +1,16 @@ +# External locations need real storage credentials with cloud IAM setup, so this +# only runs against the mock server (same as the parent test). +Cloud = false +RequiresUnityCatalog = true +# The parent test disables request recording; this one asserts on the payload. +RecordRequests = true + +# UpdateExternalLocation payload construction is direct-engine-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", + "databricks.yml.orig", +] diff --git a/acceptance/bundle/resources/external_locations/output.txt b/acceptance/bundle/resources/external_locations/output.txt index 6a6b8253eab..d228250cc50 100644 --- a/acceptance/bundle/resources/external_locations/output.txt +++ b/acceptance/bundle/resources/external_locations/output.txt @@ -14,7 +14,7 @@ Created catalogs.test_catalog Created catalogs.test_catalog.grants Created external_locations.test_location Created external_locations.test_location.grants -Files: 5 uploaded, 0 deleted +Files: 9 uploaded, 0 deleted Resources: 4 created, 0 changed, 0 deleted, 0 unchanged === Assert the catalog is created with grants diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/databricks.yml.tmpl b/acceptance/bundle/resources/registered_models/drift/comment_cleared/databricks.yml.tmpl new file mode 100644 index 00000000000..de63e932c48 --- /dev/null +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/databricks.yml.tmpl @@ -0,0 +1,14 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + schemas: + schema1: + catalog_name: main + name: test-schema-$UNIQUE_NAME + registered_models: + model1: + name: test-model-$UNIQUE_NAME + catalog_name: main + schema_name: ${resources.schemas.schema1.name} + comment: declared in the bundle diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml b/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml new file mode 100644 index 00000000000..8c52d40aa2d --- /dev/null +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt b/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt new file mode 100644 index 00000000000..84ea581d7cf --- /dev/null +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt @@ -0,0 +1,49 @@ + +=== Deploy with a comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Created registered_models.model1 +Created schemas.schema1 +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== Remove the comment from the configuration and redeploy +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Updated registered_models.model1 +Files: 0 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 1 unchanged + +=== The comment the bundle no longer declares +>>> [CLI] api get /api/2.1/unity-catalog/models/main.test-schema-[UNIQUE_NAME].test-model-[UNIQUE_NAME] +"declared in the bundle" + +=== Redeploy +>>> [CLI] bundle plan +update registered_models.model1 + +Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.registered_models.model1 + delete resources.schemas.schema1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 2 deleted + +=== The UpdateRegisteredModel payloads the deploys sent +>>> print_requests.py --method PATCH //unity-catalog/models +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/models/main.test-schema-[UNIQUE_NAME].test-model-[UNIQUE_NAME]", + "body": { + "catalog_name": "main", + "name": "test-model-[UNIQUE_NAME]", + "schema_name": "test-schema-[UNIQUE_NAME]" + } +} diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/script b/acceptance/bundle/resources/registered_models/drift/comment_cleared/script new file mode 100644 index 00000000000..d780a6df5fa --- /dev/null +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/script @@ -0,0 +1,27 @@ +echo "*" > .gitignore +envsubst < databricks.yml.tmpl > databricks.yml.in +cp databricks.yml.in databricks.yml + +MODEL="main.test-schema-${UNIQUE_NAME}.test-model-${UNIQUE_NAME}" + +# print_requests.py consumes out.requests.txt, so it runs after the last request. +cleanup() { + trace $CLI bundle destroy --auto-approve + + title "The UpdateRegisteredModel payloads the deploys sent" + trace print_requests.py --method PATCH //unity-catalog/models +} +trap cleanup EXIT + +title "Deploy with a comment" +trace $CLI bundle deploy + +title "Remove the comment from the configuration and redeploy" +grep -v 'comment:' databricks.yml.in > databricks.yml +trace $CLI bundle deploy + +title "The comment the bundle no longer declares" +MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/models/$MODEL" | jq '.comment' + +title "Redeploy" +trace $CLI bundle plan diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml b/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml new file mode 100644 index 00000000000..e7fb012fe7f --- /dev/null +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml @@ -0,0 +1,12 @@ +Cloud = true +RequiresUnityCatalog = true + +# UpdateRegisteredModel payload construction is direct-engine-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", + "databricks.yml.in", +] From 47a5c2b4e84a7a53115d76922ed9a8943e2f45b0 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 24 Aug 2026 17:52:11 +0200 Subject: [PATCH 08/16] Send the cleared comment for registered models and external locations too Apply forceSendClearedFields to the two remaining resources whose update drops a cleared field, so removing a comment from the configuration converges instead of reporting a change forever. External locations get it on both the update and the rename path. The fake workspace applies whatever an update payload names, replacing guards that swallowed an explicit empty comment. Registered models accept a payload with no field at all -- unlike schemas, catalogs and volumes, verified against a real workspace -- so parseUCUpdate is split and they use the parse-only half rather than gaining a rejection the backend does not have. Co-authored-by: Isaac --- .../schema-comment-nothing-to-update.md | 2 +- .../drift/comment_cleared/output.txt | 7 +++-- .../drift/comment_cleared/output.txt | 7 +++-- bundle/direct/dresources/external_location.go | 8 ++++-- bundle/direct/dresources/registered_model.go | 4 ++- libs/testserver/external_locations.go | 22 +++++++--------- libs/testserver/fake_workspace.go | 26 ++++++++++++++----- libs/testserver/registered_models.go | 14 +++++----- 8 files changed, 51 insertions(+), 39 deletions(-) diff --git a/.nextchanges/bundles/schema-comment-nothing-to-update.md b/.nextchanges/bundles/schema-comment-nothing-to-update.md index 7267769243a..48b969b2202 100644 --- a/.nextchanges/bundles/schema-comment-nothing-to-update.md +++ b/.nextchanges/bundles/schema-comment-nothing-to-update.md @@ -1 +1 @@ -Fixed `bundle deploy` on the direct engine failing with `Nothing to update`, or silently not converging, when a schema, catalog or volume field was set outside the bundle. The update now sends the fields the plan reports as cleared instead of dropping them. +Fixed `bundle deploy` on the direct engine failing with `Nothing to update`, or reporting a change it never applied, when a schema, catalog, volume, registered model or external location field was removed from the configuration or set outside the bundle. The update now sends the fields the plan reports as cleared instead of dropping them. diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt b/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt index e56e7686322..0ba98552707 100644 --- a/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt @@ -15,13 +15,11 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged === The comment the bundle no longer declares >>> [CLI] api get /api/2.1/unity-catalog/external-locations/myloc -"declared in the bundle" +"" === Redeploy >>> [CLI] bundle plan -update external_locations.location1 - -Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: @@ -37,6 +35,7 @@ Destroy: 1 deleted "method": "PATCH", "path": "/api/2.1/unity-catalog/external-locations/myloc", "body": { + "comment": "", "credential_name": "test_storage_credential", "url": "s3://test-bucket/path" } diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt b/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt index 84ea581d7cf..eb5310e4653 100644 --- a/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt @@ -16,13 +16,11 @@ Resources: 0 created, 1 changed, 0 deleted, 1 unchanged === The comment the bundle no longer declares >>> [CLI] api get /api/2.1/unity-catalog/models/main.test-schema-[UNIQUE_NAME].test-model-[UNIQUE_NAME] -"declared in the bundle" +"" === Redeploy >>> [CLI] bundle plan -update registered_models.model1 - -Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged +Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: @@ -43,6 +41,7 @@ Destroy: 2 deleted "path": "/api/2.1/unity-catalog/models/main.test-schema-[UNIQUE_NAME].test-model-[UNIQUE_NAME]", "body": { "catalog_name": "main", + "comment": "", "name": "test-model-[UNIQUE_NAME]", "schema_name": "test-schema-[UNIQUE_NAME]" } diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index a1ea24e4109..16fea6e8af9 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -53,7 +53,7 @@ func (r *ResourceExternalLocation) DoCreate(ctx context.Context, config *catalog } // DoUpdate updates the external location in place and returns remote state. -func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ *PlanEntry) (*catalog.ExternalLocationInfo, error) { +func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, entry *PlanEntry) (*catalog.ExternalLocationInfo, error) { updateRequest := catalog.UpdateExternalLocation{ Comment: config.Comment, CredentialName: config.CredentialName, @@ -74,11 +74,13 @@ func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, conf ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](config.ForceSendFields, "IsolationMode", "Owner"), } + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + return r.client.ExternalLocations.Update(ctx, updateRequest) } // DoUpdateWithID updates the external location and returns the new ID if the name changes. -func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ *PlanEntry) (string, *catalog.ExternalLocationInfo, error) { +func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateExternalLocation, entry *PlanEntry) (string, *catalog.ExternalLocationInfo, error) { updateRequest := catalog.UpdateExternalLocation{ Comment: config.Comment, CredentialName: config.CredentialName, @@ -103,6 +105,8 @@ func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string updateRequest.NewName = config.Name } + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + response, err := r.client.ExternalLocations.Update(ctx, updateRequest) if err != nil { return "", nil, err diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index b72038a79f0..26c4b4fb519 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -65,7 +65,7 @@ func (r *ResourceRegisteredModel) DoCreate(ctx context.Context, config *catalog. return response.FullName, response, nil } -func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ *PlanEntry) (*catalog.RegisteredModelInfo, error) { +func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, entry *PlanEntry) (*catalog.RegisteredModelInfo, error) { updateRequest := catalog.UpdateRegisteredModelRequest{ FullName: id, Comment: config.Comment, @@ -91,6 +91,8 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi CatalogName: config.CatalogName, } + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + response, err := r.client.RegisteredModels.Update(ctx, updateRequest) if err != nil { return nil, err diff --git a/libs/testserver/external_locations.go b/libs/testserver/external_locations.go index b0000deb2ad..8fba059804b 100644 --- a/libs/testserver/external_locations.go +++ b/libs/testserver/external_locations.go @@ -61,6 +61,11 @@ func (s *FakeWorkspace) ExternalLocationsUpdate(req Request, name string) Respon } } + fields, errResponse := parseUpdateFields(req.Body) + if errResponse != nil { + return *errResponse + } + var updateRequest catalog.UpdateExternalLocation if err := json.Unmarshal(req.Body, &updateRequest); err != nil { return Response{ @@ -69,19 +74,10 @@ func (s *FakeWorkspace) ExternalLocationsUpdate(req Request, name string) Respon } } - // Update only the fields that can be updated - if updateRequest.Comment != "" { - existing.Comment = updateRequest.Comment - } - if updateRequest.Url != "" { - existing.Url = updateRequest.Url - } - if updateRequest.CredentialName != "" { - existing.CredentialName = updateRequest.CredentialName - } - if updateRequest.Owner != "" { - existing.Owner = updateRequest.Owner - } + applyUpdatedFields(&existing, updateRequest, fields) + + // These three are always sent, so they track the request whether or not the payload + // names them (read_only=false and a dropped key mean the same thing here). existing.ReadOnly = updateRequest.ReadOnly existing.EnableFileEvents = updateRequest.EnableFileEvents existing.Fallback = updateRequest.Fallback diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index b568603dc73..874329ca295 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -258,13 +258,9 @@ func (s *FakeWorkspace) LockUnlock() func() { return func() { s.mu.Unlock() } } -// parseUCUpdate decodes a Unity Catalog update payload into its raw fields. It returns a -// rejection response when the body carries no field to act on: UC answers such a PATCH with -// " Nothing to update." (400) rather than treating it as a no-op. A key set to -// null does not count. Verified against a real workspace for schemas, volumes and catalogs: -// {} and {"comment": null} are rejected, while {"comment": ""} and -// {"custom_max_retention_hours": 0} are accepted. -func parseUCUpdate(body []byte, operation string) (map[string]json.RawMessage, *Response) { +// parseUpdateFields decodes an update payload into its raw fields, so a handler can tell +// a field explicitly set to a zero value from one the caller omitted. +func parseUpdateFields(body []byte) (map[string]json.RawMessage, *Response) { var fields map[string]json.RawMessage if err := json.Unmarshal(body, &fields); err != nil { @@ -273,6 +269,22 @@ func parseUCUpdate(body []byte, operation string) (map[string]json.RawMessage, * StatusCode: http.StatusInternalServerError, } } + return fields, nil +} + +// parseUCUpdate is parseUpdateFields for the UC APIs that reject a payload carrying no +// field to act on, answering " Nothing to update." (400) rather than treating it +// as a no-op. A key set to null does not count. +// +// Verified against a real workspace for schemas, volumes and catalogs: {} and +// {"comment": null} are rejected, while {"comment": ""} and +// {"custom_max_retention_hours": 0} are accepted. Registered models accept {} instead, so +// they use parseUpdateFields. +func parseUCUpdate(body []byte, operation string) (map[string]json.RawMessage, *Response) { + fields, errResponse := parseUpdateFields(body) + if errResponse != nil { + return nil, errResponse + } for _, value := range fields { if string(value) != "null" { diff --git a/libs/testserver/registered_models.go b/libs/testserver/registered_models.go index 6b6d3b4b346..b6fb800a623 100644 --- a/libs/testserver/registered_models.go +++ b/libs/testserver/registered_models.go @@ -62,6 +62,11 @@ func (s *FakeWorkspace) RegisteredModelsUpdate(req Request, fullName string) Res } } + fields, errResponse := parseUpdateFields(req.Body) + if errResponse != nil { + return *errResponse + } + var updateRequest catalog.UpdateRegisteredModelRequest if err := json.Unmarshal(req.Body, &updateRequest); err != nil { return Response{ @@ -70,13 +75,8 @@ func (s *FakeWorkspace) RegisteredModelsUpdate(req Request, fullName string) Res } } - // Update only the fields that can be updated - if updateRequest.Comment != "" { - existing.Comment = updateRequest.Comment - } - if updateRequest.Owner != "" { - existing.Owner = updateRequest.Owner - } + applyUpdatedFields(&existing, updateRequest, fields) + if updateRequest.NewName != "" { existing.Name = updateRequest.NewName From dc05154c9d6a377c78b7dbd7ed63cf012207cb46 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 24 Aug 2026 18:07:07 +0200 Subject: [PATCH 09/16] Drop RequiresUnityCatalog from the new tests main removed the setting in #6359 while this branch was open, so the merge failed with "Undecoded key ... RequiresUnityCatalog" on the direct-engine runners. Co-authored-by: Isaac --- .../resources/catalogs/drift/comment_out_of_band/out.test.toml | 1 - .../resources/catalogs/drift/comment_out_of_band/test.toml | 1 - .../external_locations/drift/comment_cleared/out.test.toml | 1 - .../resources/external_locations/drift/comment_cleared/test.toml | 1 - .../registered_models/drift/comment_cleared/out.test.toml | 1 - .../resources/registered_models/drift/comment_cleared/test.toml | 1 - .../resources/schemas/drift/comment_out_of_band/out.test.toml | 1 - .../bundle/resources/schemas/drift/comment_out_of_band/test.toml | 1 - .../resources/volumes/drift/comment_out_of_band/out.test.toml | 1 - .../bundle/resources/volumes/drift/comment_out_of_band/test.toml | 1 - 10 files changed, 10 deletions(-) diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml index 8c52d40aa2d..c502b28221b 100644 --- a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml @@ -1,3 +1,2 @@ Cloud = true -RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml index ae0b1a870d3..78d1d7f450b 100644 --- a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml +++ b/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml @@ -1,5 +1,4 @@ Cloud = true -RequiresUnityCatalog = true # UpdateCatalog payload construction is direct-engine-specific. EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml b/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml index 687990d50ea..0938e678987 100644 --- a/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml @@ -1,3 +1,2 @@ Cloud = false -RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml b/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml index f7a6c13375b..39a6fb5adae 100644 --- a/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml +++ b/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml @@ -1,7 +1,6 @@ # External locations need real storage credentials with cloud IAM setup, so this # only runs against the mock server (same as the parent test). Cloud = false -RequiresUnityCatalog = true # The parent test disables request recording; this one asserts on the payload. RecordRequests = true diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml b/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml index 8c52d40aa2d..c502b28221b 100644 --- a/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml @@ -1,3 +1,2 @@ Cloud = true -RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml b/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml index e7fb012fe7f..279916acd72 100644 --- a/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml +++ b/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml @@ -1,5 +1,4 @@ Cloud = true -RequiresUnityCatalog = true # UpdateRegisteredModel payload construction is direct-engine-specific. EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml index 8c52d40aa2d..c502b28221b 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml @@ -1,3 +1,2 @@ Cloud = true -RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml index 4f7503fc8e8..65219bba62a 100644 --- a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml +++ b/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml @@ -1,5 +1,4 @@ Cloud = true -RequiresUnityCatalog = true # UpdateSchema payload construction is direct-engine-specific. EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml index 8c52d40aa2d..c502b28221b 100644 --- a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml @@ -1,3 +1,2 @@ Cloud = true -RequiresUnityCatalog = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml index bec056028b6..27f8fb4be36 100644 --- a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml +++ b/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml @@ -1,5 +1,4 @@ Cloud = true -RequiresUnityCatalog = true # UpdateVolume payload construction is direct-engine-specific. EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] From 14a1f37381fb30d11fc12bee6059e4c2b9973487 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 25 Aug 2026 11:42:24 +0200 Subject: [PATCH 10/16] Move the new tests up out of the drift group resources// matches the equivalent tests that already live flat (cluster_policies/out_of_band_change, apps/config-drift, permissions/jobs/added_remotely), and the drift prefix says nothing the names do not. Pure rename, no content change. Co-authored-by: Isaac --- .../catalogs/{drift => }/comment_out_of_band/databricks.yml.tmpl | 0 .../catalogs/{drift => }/comment_out_of_band/out.test.toml | 0 .../resources/catalogs/{drift => }/comment_out_of_band/output.txt | 0 .../resources/catalogs/{drift => }/comment_out_of_band/script | 0 .../resources/catalogs/{drift => }/comment_out_of_band/test.toml | 0 .../external_locations/{drift => }/comment_cleared/databricks.yml | 0 .../external_locations/{drift => }/comment_cleared/out.test.toml | 0 .../external_locations/{drift => }/comment_cleared/output.txt | 0 .../external_locations/{drift => }/comment_cleared/script | 0 .../external_locations/{drift => }/comment_cleared/test.toml | 0 .../{drift => }/comment_cleared/databricks.yml.tmpl | 0 .../registered_models/{drift => }/comment_cleared/out.test.toml | 0 .../registered_models/{drift => }/comment_cleared/output.txt | 0 .../registered_models/{drift => }/comment_cleared/script | 0 .../registered_models/{drift => }/comment_cleared/test.toml | 0 .../schemas/{drift => }/comment_out_of_band/databricks.yml.tmpl | 0 .../schemas/{drift => }/comment_out_of_band/out.test.toml | 0 .../resources/schemas/{drift => }/comment_out_of_band/output.txt | 0 .../resources/schemas/{drift => }/comment_out_of_band/script | 0 .../resources/schemas/{drift => }/comment_out_of_band/test.toml | 0 .../volumes/{drift => }/comment_out_of_band/databricks.yml.tmpl | 0 .../volumes/{drift => }/comment_out_of_band/out.test.toml | 0 .../resources/volumes/{drift => }/comment_out_of_band/output.txt | 0 .../resources/volumes/{drift => }/comment_out_of_band/script | 0 .../resources/volumes/{drift => }/comment_out_of_band/test.toml | 0 25 files changed, 0 insertions(+), 0 deletions(-) rename acceptance/bundle/resources/catalogs/{drift => }/comment_out_of_band/databricks.yml.tmpl (100%) rename acceptance/bundle/resources/catalogs/{drift => }/comment_out_of_band/out.test.toml (100%) rename acceptance/bundle/resources/catalogs/{drift => }/comment_out_of_band/output.txt (100%) rename acceptance/bundle/resources/catalogs/{drift => }/comment_out_of_band/script (100%) rename acceptance/bundle/resources/catalogs/{drift => }/comment_out_of_band/test.toml (100%) rename acceptance/bundle/resources/external_locations/{drift => }/comment_cleared/databricks.yml (100%) rename acceptance/bundle/resources/external_locations/{drift => }/comment_cleared/out.test.toml (100%) rename acceptance/bundle/resources/external_locations/{drift => }/comment_cleared/output.txt (100%) rename acceptance/bundle/resources/external_locations/{drift => }/comment_cleared/script (100%) rename acceptance/bundle/resources/external_locations/{drift => }/comment_cleared/test.toml (100%) rename acceptance/bundle/resources/registered_models/{drift => }/comment_cleared/databricks.yml.tmpl (100%) rename acceptance/bundle/resources/registered_models/{drift => }/comment_cleared/out.test.toml (100%) rename acceptance/bundle/resources/registered_models/{drift => }/comment_cleared/output.txt (100%) rename acceptance/bundle/resources/registered_models/{drift => }/comment_cleared/script (100%) rename acceptance/bundle/resources/registered_models/{drift => }/comment_cleared/test.toml (100%) rename acceptance/bundle/resources/schemas/{drift => }/comment_out_of_band/databricks.yml.tmpl (100%) rename acceptance/bundle/resources/schemas/{drift => }/comment_out_of_band/out.test.toml (100%) rename acceptance/bundle/resources/schemas/{drift => }/comment_out_of_band/output.txt (100%) rename acceptance/bundle/resources/schemas/{drift => }/comment_out_of_band/script (100%) rename acceptance/bundle/resources/schemas/{drift => }/comment_out_of_band/test.toml (100%) rename acceptance/bundle/resources/volumes/{drift => }/comment_out_of_band/databricks.yml.tmpl (100%) rename acceptance/bundle/resources/volumes/{drift => }/comment_out_of_band/out.test.toml (100%) rename acceptance/bundle/resources/volumes/{drift => }/comment_out_of_band/output.txt (100%) rename acceptance/bundle/resources/volumes/{drift => }/comment_out_of_band/script (100%) rename acceptance/bundle/resources/volumes/{drift => }/comment_out_of_band/test.toml (100%) diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/catalogs/comment_out_of_band/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/resources/catalogs/drift/comment_out_of_band/databricks.yml.tmpl rename to acceptance/bundle/resources/catalogs/comment_out_of_band/databricks.yml.tmpl diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/catalogs/comment_out_of_band/out.test.toml similarity index 100% rename from acceptance/bundle/resources/catalogs/drift/comment_out_of_band/out.test.toml rename to acceptance/bundle/resources/catalogs/comment_out_of_band/out.test.toml diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt similarity index 100% rename from acceptance/bundle/resources/catalogs/drift/comment_out_of_band/output.txt rename to acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/script b/acceptance/bundle/resources/catalogs/comment_out_of_band/script similarity index 100% rename from acceptance/bundle/resources/catalogs/drift/comment_out_of_band/script rename to acceptance/bundle/resources/catalogs/comment_out_of_band/script diff --git a/acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/catalogs/comment_out_of_band/test.toml similarity index 100% rename from acceptance/bundle/resources/catalogs/drift/comment_out_of_band/test.toml rename to acceptance/bundle/resources/catalogs/comment_out_of_band/test.toml diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/databricks.yml b/acceptance/bundle/resources/external_locations/comment_cleared/databricks.yml similarity index 100% rename from acceptance/bundle/resources/external_locations/drift/comment_cleared/databricks.yml rename to acceptance/bundle/resources/external_locations/comment_cleared/databricks.yml diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml b/acceptance/bundle/resources/external_locations/comment_cleared/out.test.toml similarity index 100% rename from acceptance/bundle/resources/external_locations/drift/comment_cleared/out.test.toml rename to acceptance/bundle/resources/external_locations/comment_cleared/out.test.toml diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt b/acceptance/bundle/resources/external_locations/comment_cleared/output.txt similarity index 100% rename from acceptance/bundle/resources/external_locations/drift/comment_cleared/output.txt rename to acceptance/bundle/resources/external_locations/comment_cleared/output.txt diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/script b/acceptance/bundle/resources/external_locations/comment_cleared/script similarity index 100% rename from acceptance/bundle/resources/external_locations/drift/comment_cleared/script rename to acceptance/bundle/resources/external_locations/comment_cleared/script diff --git a/acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml b/acceptance/bundle/resources/external_locations/comment_cleared/test.toml similarity index 100% rename from acceptance/bundle/resources/external_locations/drift/comment_cleared/test.toml rename to acceptance/bundle/resources/external_locations/comment_cleared/test.toml diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/databricks.yml.tmpl b/acceptance/bundle/resources/registered_models/comment_cleared/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/resources/registered_models/drift/comment_cleared/databricks.yml.tmpl rename to acceptance/bundle/resources/registered_models/comment_cleared/databricks.yml.tmpl diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml b/acceptance/bundle/resources/registered_models/comment_cleared/out.test.toml similarity index 100% rename from acceptance/bundle/resources/registered_models/drift/comment_cleared/out.test.toml rename to acceptance/bundle/resources/registered_models/comment_cleared/out.test.toml diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt b/acceptance/bundle/resources/registered_models/comment_cleared/output.txt similarity index 100% rename from acceptance/bundle/resources/registered_models/drift/comment_cleared/output.txt rename to acceptance/bundle/resources/registered_models/comment_cleared/output.txt diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/script b/acceptance/bundle/resources/registered_models/comment_cleared/script similarity index 100% rename from acceptance/bundle/resources/registered_models/drift/comment_cleared/script rename to acceptance/bundle/resources/registered_models/comment_cleared/script diff --git a/acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml b/acceptance/bundle/resources/registered_models/comment_cleared/test.toml similarity index 100% rename from acceptance/bundle/resources/registered_models/drift/comment_cleared/test.toml rename to acceptance/bundle/resources/registered_models/comment_cleared/test.toml diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/schemas/comment_out_of_band/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/resources/schemas/drift/comment_out_of_band/databricks.yml.tmpl rename to acceptance/bundle/resources/schemas/comment_out_of_band/databricks.yml.tmpl diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/schemas/comment_out_of_band/out.test.toml similarity index 100% rename from acceptance/bundle/resources/schemas/drift/comment_out_of_band/out.test.toml rename to acceptance/bundle/resources/schemas/comment_out_of_band/out.test.toml diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt similarity index 100% rename from acceptance/bundle/resources/schemas/drift/comment_out_of_band/output.txt rename to acceptance/bundle/resources/schemas/comment_out_of_band/output.txt diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/script b/acceptance/bundle/resources/schemas/comment_out_of_band/script similarity index 100% rename from acceptance/bundle/resources/schemas/drift/comment_out_of_band/script rename to acceptance/bundle/resources/schemas/comment_out_of_band/script diff --git a/acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/schemas/comment_out_of_band/test.toml similarity index 100% rename from acceptance/bundle/resources/schemas/drift/comment_out_of_band/test.toml rename to acceptance/bundle/resources/schemas/comment_out_of_band/test.toml diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl b/acceptance/bundle/resources/volumes/comment_out_of_band/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/resources/volumes/drift/comment_out_of_band/databricks.yml.tmpl rename to acceptance/bundle/resources/volumes/comment_out_of_band/databricks.yml.tmpl diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml b/acceptance/bundle/resources/volumes/comment_out_of_band/out.test.toml similarity index 100% rename from acceptance/bundle/resources/volumes/drift/comment_out_of_band/out.test.toml rename to acceptance/bundle/resources/volumes/comment_out_of_band/out.test.toml diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt b/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt similarity index 100% rename from acceptance/bundle/resources/volumes/drift/comment_out_of_band/output.txt rename to acceptance/bundle/resources/volumes/comment_out_of_band/output.txt diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/script b/acceptance/bundle/resources/volumes/comment_out_of_band/script similarity index 100% rename from acceptance/bundle/resources/volumes/drift/comment_out_of_band/script rename to acceptance/bundle/resources/volumes/comment_out_of_band/script diff --git a/acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml b/acceptance/bundle/resources/volumes/comment_out_of_band/test.toml similarity index 100% rename from acceptance/bundle/resources/volumes/drift/comment_out_of_band/test.toml rename to acceptance/bundle/resources/volumes/comment_out_of_band/test.toml From 47837f05a5f2e2281988d08293b3df8e20b47aed Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 25 Aug 2026 11:54:38 +0200 Subject: [PATCH 11/16] Edit the config with update_file.py instead of grep -v grep -v silently leaves the config unchanged if the pattern stops matching, and these tests would then assert convergence against a config that still declares the comment. update_file.py fails instead, and the intermediate copies of databricks.yml are no longer needed. Co-authored-by: Isaac --- .../resources/external_locations/comment_cleared/script | 3 +-- .../resources/external_locations/comment_cleared/test.toml | 1 - .../resources/registered_models/comment_cleared/script | 5 ++--- .../resources/registered_models/comment_cleared/test.toml | 1 - 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/acceptance/bundle/resources/external_locations/comment_cleared/script b/acceptance/bundle/resources/external_locations/comment_cleared/script index 0a5871d39f1..f5a82dc9552 100644 --- a/acceptance/bundle/resources/external_locations/comment_cleared/script +++ b/acceptance/bundle/resources/external_locations/comment_cleared/script @@ -1,5 +1,4 @@ echo "*" > .gitignore -cp databricks.yml databricks.yml.orig # print_requests.py consumes out.requests.txt, so it runs after the last request. cleanup() { @@ -14,7 +13,7 @@ title "Deploy with a comment" trace $CLI bundle deploy title "Remove the comment from the configuration and redeploy" -grep -v 'comment:' databricks.yml.orig > databricks.yml +update_file.py databricks.yml 'comment: declared in the bundle' '' trace $CLI bundle deploy title "The comment the bundle no longer declares" diff --git a/acceptance/bundle/resources/external_locations/comment_cleared/test.toml b/acceptance/bundle/resources/external_locations/comment_cleared/test.toml index 39a6fb5adae..31bed342069 100644 --- a/acceptance/bundle/resources/external_locations/comment_cleared/test.toml +++ b/acceptance/bundle/resources/external_locations/comment_cleared/test.toml @@ -11,5 +11,4 @@ Ignore = [ ".databricks", ".gitignore", "databricks.yml", - "databricks.yml.orig", ] diff --git a/acceptance/bundle/resources/registered_models/comment_cleared/script b/acceptance/bundle/resources/registered_models/comment_cleared/script index d780a6df5fa..f466c7f6422 100644 --- a/acceptance/bundle/resources/registered_models/comment_cleared/script +++ b/acceptance/bundle/resources/registered_models/comment_cleared/script @@ -1,6 +1,5 @@ echo "*" > .gitignore -envsubst < databricks.yml.tmpl > databricks.yml.in -cp databricks.yml.in databricks.yml +envsubst < databricks.yml.tmpl > databricks.yml MODEL="main.test-schema-${UNIQUE_NAME}.test-model-${UNIQUE_NAME}" @@ -17,7 +16,7 @@ title "Deploy with a comment" trace $CLI bundle deploy title "Remove the comment from the configuration and redeploy" -grep -v 'comment:' databricks.yml.in > databricks.yml +update_file.py databricks.yml 'comment: declared in the bundle' '' trace $CLI bundle deploy title "The comment the bundle no longer declares" diff --git a/acceptance/bundle/resources/registered_models/comment_cleared/test.toml b/acceptance/bundle/resources/registered_models/comment_cleared/test.toml index 279916acd72..3ad98d57646 100644 --- a/acceptance/bundle/resources/registered_models/comment_cleared/test.toml +++ b/acceptance/bundle/resources/registered_models/comment_cleared/test.toml @@ -7,5 +7,4 @@ Ignore = [ ".databricks", ".gitignore", "databricks.yml", - "databricks.yml.in", ] From 6c0805170035e43da5dbe267890e6a195e26b51a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 25 Aug 2026 12:21:50 +0200 Subject: [PATCH 12/16] Force-send cleared fields on the catalog and volume rename paths too External locations already did this; catalogs and volumes did not, so a deploy that renamed one of them dropped a field the plan reported as cleared. The stale comment left behind claimed DoUpdateWithID cannot see the plan, which stopped being true when #6360 landed. Co-authored-by: Isaac --- .../comment_with_rename/databricks.yml.tmpl | 13 +++++ .../volumes/comment_with_rename/out.test.toml | 2 + .../volumes/comment_with_rename/output.txt | 58 +++++++++++++++++++ .../volumes/comment_with_rename/script | 25 ++++++++ .../volumes/comment_with_rename/test.toml | 10 ++++ bundle/direct/dresources/catalog.go | 7 +-- bundle/direct/dresources/volume.go | 5 +- 7 files changed, 114 insertions(+), 6 deletions(-) create mode 100644 acceptance/bundle/resources/volumes/comment_with_rename/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/volumes/comment_with_rename/out.test.toml create mode 100644 acceptance/bundle/resources/volumes/comment_with_rename/output.txt create mode 100644 acceptance/bundle/resources/volumes/comment_with_rename/script create mode 100644 acceptance/bundle/resources/volumes/comment_with_rename/test.toml diff --git a/acceptance/bundle/resources/volumes/comment_with_rename/databricks.yml.tmpl b/acceptance/bundle/resources/volumes/comment_with_rename/databricks.yml.tmpl new file mode 100644 index 00000000000..9c7779e59f2 --- /dev/null +++ b/acceptance/bundle/resources/volumes/comment_with_rename/databricks.yml.tmpl @@ -0,0 +1,13 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + schemas: + schema1: + catalog_name: main + name: test-schema-$UNIQUE_NAME + volumes: + volume1: + catalog_name: main + schema_name: ${resources.schemas.schema1.name} + name: vol_one diff --git a/acceptance/bundle/resources/volumes/comment_with_rename/out.test.toml b/acceptance/bundle/resources/volumes/comment_with_rename/out.test.toml new file mode 100644 index 00000000000..c502b28221b --- /dev/null +++ b/acceptance/bundle/resources/volumes/comment_with_rename/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/volumes/comment_with_rename/output.txt b/acceptance/bundle/resources/volumes/comment_with_rename/output.txt new file mode 100644 index 00000000000..3713e45f084 --- /dev/null +++ b/acceptance/bundle/resources/volumes/comment_with_rename/output.txt @@ -0,0 +1,58 @@ + +=== Deploy a volume that does not declare a comment +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Created schemas.schema1 +Created volumes.volume1 +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== Set the comment out of band, then rename the volume in the same deploy +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Updated volumes.volume1 +Files: 0 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 1 unchanged + +=== The rename and the clear both landed, so it converges in one pass +>>> [CLI] api get /api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].vol_two +"" + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.schema1 + delete resources.volumes.volume1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +This action will result in the deletion of the following volumes. +For managed volumes, the files stored in the volume are also deleted from your +cloud tenant within 30 days. For external volumes, the metadata about the volume +is removed from the catalog, but the underlying files are not deleted: + delete resources.volumes.volume1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 2 deleted + +=== The rename carried the cleared comment alongside new_name +>>> print_requests.py --method PATCH //unity-catalog/volumes +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].vol_one", + "body": { + "comment": "set outside the bundle" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].vol_one", + "body": { + "comment": "", + "new_name": "vol_two" + } +} diff --git a/acceptance/bundle/resources/volumes/comment_with_rename/script b/acceptance/bundle/resources/volumes/comment_with_rename/script new file mode 100644 index 00000000000..5229e4ef5c8 --- /dev/null +++ b/acceptance/bundle/resources/volumes/comment_with_rename/script @@ -0,0 +1,25 @@ +echo "*" > .gitignore +envsubst < databricks.yml.tmpl > databricks.yml + +VOLUME="main.test-schema-${UNIQUE_NAME}.vol_one" + +# print_requests.py consumes out.requests.txt, so it runs after the last request. +cleanup() { + trace $CLI bundle destroy --auto-approve + + title "The rename carried the cleared comment alongside new_name" + trace print_requests.py --method PATCH //unity-catalog/volumes +} +trap cleanup EXIT + +title "Deploy a volume that does not declare a comment" +trace $CLI bundle deploy + +title "Set the comment out of band, then rename the volume in the same deploy" +MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/volumes/$VOLUME" --json '{"comment":"set outside the bundle"}' > /dev/null +update_file.py databricks.yml 'name: vol_one' 'name: vol_two' +trace $CLI bundle deploy + +title "The rename and the clear both landed, so it converges in one pass" +MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/volumes/main.test-schema-${UNIQUE_NAME}.vol_two" | jq '.comment' +trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged" diff --git a/acceptance/bundle/resources/volumes/comment_with_rename/test.toml b/acceptance/bundle/resources/volumes/comment_with_rename/test.toml new file mode 100644 index 00000000000..27f8fb4be36 --- /dev/null +++ b/acceptance/bundle/resources/volumes/comment_with_rename/test.toml @@ -0,0 +1,10 @@ +Cloud = true + +# UpdateVolume payload construction is direct-engine-specific. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", +] diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index ec3691d213d..a18b3dd6547 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -76,7 +76,7 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal } // DoUpdateWithID updates the catalog and returns the new ID if the name changes. -func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateCatalog, _ *PlanEntry) (string, *catalog.CatalogInfo, error) { +func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateCatalog, entry *PlanEntry) (string, *catalog.CatalogInfo, error) { updateRequest := catalog.UpdateCatalog{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -95,9 +95,8 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config updateRequest.NewName = config.Name } - // No forceSendClearedFields here: the rename always carries new_name, so the payload is - // never empty, and DoUpdateWithID has no PlanEntry to consult. A field cleared in the - // same deploy as a rename is picked up by the DoUpdate on the next one. + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { return "", nil, err diff --git a/bundle/direct/dresources/volume.go b/bundle/direct/dresources/volume.go index 09cfedfe74d..0403c2c020f 100644 --- a/bundle/direct/dresources/volume.go +++ b/bundle/direct/dresources/volume.go @@ -81,7 +81,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo return response, err } -func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, _ *PlanEntry) (string, *catalog.VolumeInfo, error) { +func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, entry *PlanEntry) (string, *catalog.VolumeInfo, error) { updateRequest := catalog.UpdateVolumeRequestContent{ Comment: config.Comment, Name: id, @@ -102,7 +102,8 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * updateRequest.NewName = config.Name } - // See ResourceCatalog.DoUpdateWithID on why the rename path does not force-send. + updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil || response == nil { return "", nil, err From f37d39fd0c2ac2ca3cc38053ba13455373c0395d Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 25 Aug 2026 14:47:37 +0200 Subject: [PATCH 13/16] Do not force-send fields the update deliberately excludes An adversarial review found that removing a config-set owner from a registered model made the plan report an actionable change, so the cleared value was force-sent as owner: "" -- which UC rejects with "Could not find principal with name .". backend_defaults does not cover it: that rule only skips when old and new are both nil. Route the cleared fields through the same FilterFields exclusions as config.ForceSendFields, so the exclusion list each request already declares is authoritative for both. The comment claiming owner is not in the configuration tree was wrong: it is, via the embedded CreateRegisteredModelRequest. Also record the whole changes map in the drift goldens rather than just comment, and stop the fake workspace accumulating duplicate ForceSendFields entries. Co-authored-by: Isaac --- .../resources/catalogs/comment_out_of_band/output.txt | 6 ++++-- .../resources/catalogs/comment_out_of_band/script | 2 +- .../resources/schemas/comment_out_of_band/output.txt | 6 ++++-- .../resources/schemas/comment_out_of_band/script | 2 +- .../resources/volumes/comment_out_of_band/output.txt | 11 +++++++++-- .../resources/volumes/comment_out_of_band/script | 2 +- bundle/direct/dresources/catalog.go | 10 ++++++---- bundle/direct/dresources/external_location.go | 10 ++++++---- bundle/direct/dresources/registered_model.go | 9 ++++++--- bundle/direct/dresources/schema.go | 5 +++-- bundle/direct/dresources/volume.go | 10 ++++++---- libs/testserver/fake_workspace.go | 6 +++++- 12 files changed, 52 insertions(+), 27 deletions(-) diff --git a/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt b/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt index 6ff82f588e6..ec723ea3a14 100644 --- a/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt @@ -10,8 +10,10 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged === The remote comment is drift, so the plan updates the catalog >>> [CLI] bundle plan --output json { - "action": "update", - "remote": "set outside the bundle" + "comment": { + "action": "update", + "remote": "set outside the bundle" + } } === Redeploy clears the comment diff --git a/acceptance/bundle/resources/catalogs/comment_out_of_band/script b/acceptance/bundle/resources/catalogs/comment_out_of_band/script index 6f1a938177d..dafb61ac5fb 100644 --- a/acceptance/bundle/resources/catalogs/comment_out_of_band/script +++ b/acceptance/bundle/resources/catalogs/comment_out_of_band/script @@ -20,7 +20,7 @@ title "Set the comment out of band, the way Catalog Explorer does" MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/catalogs/$CATALOG" --json '{"comment":"set outside the bundle"}' > /dev/null title "The remote comment is drift, so the plan updates the catalog" -trace $CLI bundle plan --output json | jq '.plan[].changes.comment' +trace $CLI bundle plan --output json | jq '.plan[].changes' title "Redeploy clears the comment" trace $CLI bundle deploy diff --git a/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt index bffdb9c8d36..02193799389 100644 --- a/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt @@ -10,8 +10,10 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged === The remote comment is drift, so the plan updates the schema >>> [CLI] bundle plan --output json { - "action": "update", - "remote": "set outside the bundle" + "comment": { + "action": "update", + "remote": "set outside the bundle" + } } === Redeploy clears the comment diff --git a/acceptance/bundle/resources/schemas/comment_out_of_band/script b/acceptance/bundle/resources/schemas/comment_out_of_band/script index 3c2514950e0..7b71c20a846 100644 --- a/acceptance/bundle/resources/schemas/comment_out_of_band/script +++ b/acceptance/bundle/resources/schemas/comment_out_of_band/script @@ -20,7 +20,7 @@ title "Set the comment out of band, the way Catalog Explorer does" MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/schemas/$SCHEMA" --json '{"comment":"set outside the bundle"}' > /dev/null title "The remote comment is drift, so the plan updates the schema" -trace $CLI bundle plan --output json | jq '.plan[].changes.comment' +trace $CLI bundle plan --output json | jq '.plan[].changes' title "Redeploy clears the comment" trace $CLI bundle deploy diff --git a/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt b/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt index c1e1f119fa1..a48c5501c24 100644 --- a/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt @@ -11,8 +11,15 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged === The remote comment is drift, so the plan updates the volume >>> [CLI] bundle plan --output json { - "action": "update", - "remote": "set outside the bundle" + "comment": { + "action": "update", + "remote": "set outside the bundle" + }, + "storage_location": { + "action": "skip", + "reason": "backend_default", + "remote": "s3://[METASTORE_NAME]/metastore/[UUID]/volumes/[UUID]" + } } === Redeploy clears the comment diff --git a/acceptance/bundle/resources/volumes/comment_out_of_band/script b/acceptance/bundle/resources/volumes/comment_out_of_band/script index c176403ba00..7b37d129aed 100644 --- a/acceptance/bundle/resources/volumes/comment_out_of_band/script +++ b/acceptance/bundle/resources/volumes/comment_out_of_band/script @@ -20,7 +20,7 @@ title "Set the comment out of band, the way Catalog Explorer does" MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/volumes/$VOLUME" --json '{"comment":"set outside the bundle"}' > /dev/null title "The remote comment is drift, so the plan updates the volume" -trace $CLI bundle plan --output json | jq '.plan["resources.volumes.volume1"].changes.comment' +trace $CLI bundle plan --output json | jq '.plan["resources.volumes.volume1"].changes' title "Redeploy clears the comment" trace $CLI bundle deploy diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index a18b3dd6547..8edeb5f3d78 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -62,10 +62,11 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(cleared, config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { @@ -88,14 +89,15 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions } if config.Name != id { updateRequest.NewName = config.Name } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(cleared, config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index 16fea6e8af9..7d364574f1d 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -71,10 +71,11 @@ func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, conf ReadOnly: config.ReadOnly, SkipValidation: config.SkipValidation, Url: config.Url, - ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](config.ForceSendFields, "IsolationMode", "Owner"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(cleared, config.ForceSendFields...), "IsolationMode", "Owner") return r.client.ExternalLocations.Update(ctx, updateRequest) } @@ -98,14 +99,15 @@ func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string ReadOnly: config.ReadOnly, SkipValidation: config.SkipValidation, Url: config.Url, - ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](config.ForceSendFields, "IsolationMode", "Owner"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions } if config.Name != id { updateRequest.NewName = config.Name } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(cleared, config.ForceSendFields...), "IsolationMode", "Owner") response, err := r.client.ExternalLocations.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 26c4b4fb519..eabf7667b02 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -69,9 +69,11 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi updateRequest := catalog.UpdateRegisteredModelRequest{ FullName: id, Comment: config.Comment, - ForceSendFields: utils.FilterFields[catalog.UpdateRegisteredModelRequest](config.ForceSendFields, "Owner", "NewName"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions - // Owner is not part of the configuration tree + // Owner is settable in the config (it comes from the embedded + // CreateRegisteredModelRequest) and create sends it, but update never has: a + // change to it is silently dropped rather than applied. Owner: "", // Name updates are not supported yet without recreating. Can be added as a follow-up. @@ -91,7 +93,8 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi CatalogName: config.CatalogName, } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateRegisteredModelRequest](append(cleared, config.ForceSendFields...), "Owner", "NewName") response, err := r.client.RegisteredModels.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/schema.go b/bundle/direct/dresources/schema.go index 6f77d19e7e2..a26e92594f9 100644 --- a/bundle/direct/dresources/schema.go +++ b/bundle/direct/dresources/schema.go @@ -56,10 +56,11 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // We recreate schemas on name change intentionally. Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateSchema](config.ForceSendFields, "EnablePredictiveOptimization", "NewName", "Owner"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateSchema](append(cleared, config.ForceSendFields...), "EnablePredictiveOptimization", "NewName", "Owner") response, err := r.client.Schemas.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/volume.go b/bundle/direct/dresources/volume.go index 0403c2c020f..9ebc06ff962 100644 --- a/bundle/direct/dresources/volume.go +++ b/bundle/direct/dresources/volume.go @@ -55,7 +55,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // Not supported by Update(). Needs DoUpdateWithID() Owner: "", // Not supported by DABs - ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "NewName", "Owner"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions } nameFromID, err := getNameFromID(id) @@ -67,7 +67,8 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo return nil, fmt.Errorf("internal error: unexpected change of name from %#v to %#v", nameFromID, config.Name) } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(cleared, config.ForceSendFields...), "NewName", "Owner") response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil { @@ -89,7 +90,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * NewName: "", // Initialized below if needed Owner: "", // Not supported by DABs - ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "Owner"), + ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions } items := strings.Split(id, ".") @@ -102,7 +103,8 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * updateRequest.NewName = config.Name } - updateRequest.ForceSendFields = append(updateRequest.ForceSendFields, forceSendClearedFields(&updateRequest, entry.Changes)...) + cleared := forceSendClearedFields(&updateRequest, entry.Changes) + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(cleared, config.ForceSendFields...), "Owner") response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil || response == nil { diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 874329ca295..2f077b1b631 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -328,7 +328,11 @@ func applyUpdatedFields(existing, update any, fields map[string]json.RawMessage) dstField.Set(src.Field(i)) forceSend := dst.FieldByName("ForceSendFields") if forceSend.IsValid() && forceSend.CanSet() { - forceSend.Set(reflect.Append(forceSend, reflect.ValueOf(src.Type().Field(i).Name))) + goName := src.Type().Field(i).Name + // Repeated updates would otherwise keep appending the same name. + if !slices.Contains(forceSend.Interface().([]string), goName) { + forceSend.Set(reflect.Append(forceSend, reflect.ValueOf(goName))) + } } } } From 8fb90d27893558a51beb57e2b583dfda978931ad Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 25 Aug 2026 16:01:32 +0200 Subject: [PATCH 14/16] Declare the fields to force-send instead of deriving them from the plan Deriving from entry.Changes made every omitempty field a candidate, so the default for an unverified field was to send its zero value -- the hazard #6088 was about ('' is not a valid cluster policy ID). It also went against dresources/README.md, which asks for a static list rather than field names derived from the plan. Each resource now names the fields it force-sends, and only comment qualifies. Three things have to hold, and the last one is easy to miss: the backend must accept the zero value as a clear (rules out owner and new_name), ForceSendFields must affect the field at all (rules out maps such as properties), and terraform must send it too (rules out custom_max_retention_hours, which UC does clear on 0 but terraform never sends, so force-sending it made the engines disagree). Co-authored-by: Isaac --- .../schemas/comment_out_of_band/output.txt | 28 ------------ .../schemas/comment_out_of_band/script | 8 ---- bundle/direct/dresources/catalog.go | 18 ++++---- bundle/direct/dresources/external_location.go | 22 ++++++---- bundle/direct/dresources/registered_model.go | 11 +++-- bundle/direct/dresources/schema.go | 24 +++++++++-- bundle/direct/dresources/util.go | 43 ------------------- bundle/direct/dresources/volume.go | 18 ++++---- 8 files changed, 61 insertions(+), 111 deletions(-) diff --git a/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt index 02193799389..b20e165a091 100644 --- a/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt @@ -33,20 +33,6 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQU Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 1 unchanged -=== The same holds for a non-string field set out of band ->>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... -Updated schemas.schema1 -Files: 0 uploaded, 0 deleted -Resources: 0 created, 1 changed, 0 deleted, 0 unchanged - ->>> [CLI] api get /api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME] -0 - -=== And it converges ->>> [CLI] bundle plan -Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged - >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: delete resources.schemas.schema1 @@ -74,17 +60,3 @@ Destroy: 1 deleted "comment": "" } } -{ - "method": "PATCH", - "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", - "body": { - "custom_max_retention_hours": 240 - } -} -{ - "method": "PATCH", - "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", - "body": { - "custom_max_retention_hours": 0 - } -} diff --git a/acceptance/bundle/resources/schemas/comment_out_of_band/script b/acceptance/bundle/resources/schemas/comment_out_of_band/script index 7b71c20a846..e0726bd56a2 100644 --- a/acceptance/bundle/resources/schemas/comment_out_of_band/script +++ b/acceptance/bundle/resources/schemas/comment_out_of_band/script @@ -31,11 +31,3 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | title "Redeploy is a no-op" trace $CLI bundle deploy -# 240h (10 days) is inside UC's valid 7-30 day window; smaller values are rejected. -title "The same holds for a non-string field set out of band" -MSYS_NO_PATHCONV=1 $CLI api patch "/api/2.1/unity-catalog/schemas/$SCHEMA" --json '{"custom_max_retention_hours":240}' > /dev/null -trace $CLI bundle deploy -MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | jq '.custom_max_retention_hours' - -title "And it converges" -trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged" diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 8edeb5f3d78..17b36a2e069 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -2,6 +2,7 @@ package dresources import ( "context" + "slices" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/utils" @@ -49,8 +50,11 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa return response.Name, response, nil } +// See schemaForceSend. Verified against a real workspace: {"comment": ""} clears it. +var catalogForceSend = []string{"Comment"} + // DoUpdate updates the catalog in place and returns remote state. -func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, entry *PlanEntry) (*catalog.CatalogInfo, error) { +func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, _ *PlanEntry) (*catalog.CatalogInfo, error) { updateRequest := catalog.UpdateCatalog{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -62,11 +66,10 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(cleared, config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(slices.Clone(catalogForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { @@ -77,7 +80,7 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal } // DoUpdateWithID updates the catalog and returns the new ID if the name changes. -func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateCatalog, entry *PlanEntry) (string, *catalog.CatalogInfo, error) { +func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateCatalog, _ *PlanEntry) (string, *catalog.CatalogInfo, error) { updateRequest := catalog.UpdateCatalog{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -89,15 +92,14 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below } if config.Name != id { updateRequest.NewName = config.Name } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(cleared, config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(slices.Clone(catalogForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index 7d364574f1d..fedcb606ecb 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -2,6 +2,7 @@ package dresources import ( "context" + "slices" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/utils" @@ -52,8 +53,15 @@ func (r *ResourceExternalLocation) DoCreate(ctx context.Context, config *catalog return response.Name, response, nil } +// See schemaForceSend. Unlike the other four this one is not probed: an external +// location needs a storage credential with cloud IAM setup, which the test workspaces do +// not provision, so its acceptance test is local-only. Comment is carried here because it +// is the same field on the same UC PATCH family, where all four siblings clear on +// {"comment": ""}; re-probe before adding anything else. +var externalLocationForceSend = []string{"Comment"} + // DoUpdate updates the external location in place and returns remote state. -func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, entry *PlanEntry) (*catalog.ExternalLocationInfo, error) { +func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ *PlanEntry) (*catalog.ExternalLocationInfo, error) { updateRequest := catalog.UpdateExternalLocation{ Comment: config.Comment, CredentialName: config.CredentialName, @@ -71,17 +79,16 @@ func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, conf ReadOnly: config.ReadOnly, SkipValidation: config.SkipValidation, Url: config.Url, - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(cleared, config.ForceSendFields...), "IsolationMode", "Owner") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(slices.Clone(externalLocationForceSend), config.ForceSendFields...), "IsolationMode", "Owner") return r.client.ExternalLocations.Update(ctx, updateRequest) } // DoUpdateWithID updates the external location and returns the new ID if the name changes. -func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateExternalLocation, entry *PlanEntry) (string, *catalog.ExternalLocationInfo, error) { +func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ *PlanEntry) (string, *catalog.ExternalLocationInfo, error) { updateRequest := catalog.UpdateExternalLocation{ Comment: config.Comment, CredentialName: config.CredentialName, @@ -99,15 +106,14 @@ func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string ReadOnly: config.ReadOnly, SkipValidation: config.SkipValidation, Url: config.Url, - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below } if config.Name != id { updateRequest.NewName = config.Name } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(cleared, config.ForceSendFields...), "IsolationMode", "Owner") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(slices.Clone(externalLocationForceSend), config.ForceSendFields...), "IsolationMode", "Owner") response, err := r.client.ExternalLocations.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index eabf7667b02..f3d708ff786 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -2,6 +2,7 @@ package dresources import ( "context" + "slices" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/utils" @@ -65,11 +66,14 @@ func (r *ResourceRegisteredModel) DoCreate(ctx context.Context, config *catalog. return response.FullName, response, nil } -func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, entry *PlanEntry) (*catalog.RegisteredModelInfo, error) { +// See schemaForceSend. Verified against a real workspace: {"comment": ""} clears it. +var registeredModelForceSend = []string{"Comment"} + +func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ *PlanEntry) (*catalog.RegisteredModelInfo, error) { updateRequest := catalog.UpdateRegisteredModelRequest{ FullName: id, Comment: config.Comment, - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below // Owner is settable in the config (it comes from the embedded // CreateRegisteredModelRequest) and create sends it, but update never has: a @@ -93,8 +97,7 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi CatalogName: config.CatalogName, } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateRegisteredModelRequest](append(cleared, config.ForceSendFields...), "Owner", "NewName") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateRegisteredModelRequest](append(slices.Clone(registeredModelForceSend), config.ForceSendFields...), "Owner", "NewName") response, err := r.client.RegisteredModels.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/schema.go b/bundle/direct/dresources/schema.go index a26e92594f9..872b2b26dc4 100644 --- a/bundle/direct/dresources/schema.go +++ b/bundle/direct/dresources/schema.go @@ -2,6 +2,7 @@ package dresources import ( "context" + "slices" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/log" @@ -46,8 +47,24 @@ func (r *ResourceSchema) DoCreate(ctx context.Context, config *catalog.CreateSch return response.FullName, response, nil } +// UC drops a field the payload omits, so a value the config stops declaring never +// reaches the backend and every later plan reports the same pending change. Worse, when +// nothing else is in the payload UC rejects it outright with "Nothing to update" (400), +// which made a schema undeployable once someone set a description on it outside the +// bundle. Sending these explicitly is the only way to express "the bundle owns this". +// +// Only fields verified against a real workspace belong here: {"comment": ""} clears it. Do +// not add a field without probing it -- UC rejects an empty value on some (owner: "" is +// "Could not find principal with name .", new_name: "" is "not a valid name"), and +// ForceSendFields is inert for maps, so listing properties would be a no-op. +// +// custom_max_retention_hours is deliberately absent even though {"...": 0} does clear it: +// terraform never sends it, so force-sending would make the two engines produce different +// payloads for the same config. +var schemaForceSend = []string{"Comment"} + // DoUpdate updates the schema in place and returns remote state. -func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalog.CreateSchema, entry *PlanEntry) (*catalog.SchemaInfo, error) { +func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalog.CreateSchema, _ *PlanEntry) (*catalog.SchemaInfo, error) { updateRequest := catalog.UpdateSchema{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -56,11 +73,10 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // We recreate schemas on name change intentionally. Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateSchema](append(cleared, config.ForceSendFields...), "EnablePredictiveOptimization", "NewName", "Owner") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateSchema](append(slices.Clone(schemaForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "NewName", "Owner") response, err := r.client.Schemas.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/util.go b/bundle/direct/dresources/util.go index 9a10ecce691..fc68f3ea37c 100644 --- a/bundle/direct/dresources/util.go +++ b/bundle/direct/dresources/util.go @@ -2,13 +2,10 @@ package dresources import ( "errors" - "reflect" "slices" "strings" "github.com/databricks/cli/bundle/deployplan" - "github.com/databricks/cli/libs/structs/structpath" - "github.com/databricks/cli/libs/structs/structtag" "github.com/databricks/databricks-sdk-go/retries" ) @@ -68,43 +65,3 @@ func collectLeafUpdatePathsWithPrefix(changes Changes, prefix string) []string { slices.Sort(paths) return paths } - -// forceSendClearedFields returns ForceSendFields entries for the request fields the plan -// is clearing: the plan reports a change, but the value is zero, so omitempty would drop -// the field from the payload entirely. -// -// A partial-update API reads an absent field as "leave unchanged", so dropping it -// silently discards the clear and the next plan reports the same drift forever. Unity -// Catalog goes further and rejects a payload carrying no field at all with -// " Nothing to update." (400), which made a schema, catalog or volume -// undeployable as soon as someone set a description on it outside the bundle (#6340). -// -// Every omitempty field assigned from config into a partial-update request shares the -// problem -- roughly 40 of them across the UC resources, not just comment -- so this is -// driven off the plan instead of naming individual fields. Resources whose update -// replaces the whole object (jobs, pipelines, model serving) need none of it: there an -// absent field already means cleared, and they converge on their own. -// -// Matching is by the request type's own JSON names, so a field the request does not -// expose, or names differently (e.g. new_name), is never force-sent. Skipped changes are -// excluded by HasChange, so a field the plan suppressed -- via backend_defaults or -// ignore_remote_changes -- is left alone rather than being cleared behind the plan's back. -func forceSendClearedFields(request any, changes Changes) []string { - value := reflect.ValueOf(request).Elem() - valueType := value.Type() - - var result []string - for i := range valueType.NumField() { - field := valueType.Field(i) - name := structtag.JSONTag(field.Tag.Get("json")).Name() - if name == "" || name == "-" || !value.Field(i).IsZero() { - continue - } - if changes.HasChange(structpath.NewDotString(nil, name)) { - result = append(result, field.Name) - } - } - - slices.Sort(result) - return result -} diff --git a/bundle/direct/dresources/volume.go b/bundle/direct/dresources/volume.go index 9ebc06ff962..2c6967a7998 100644 --- a/bundle/direct/dresources/volume.go +++ b/bundle/direct/dresources/volume.go @@ -3,6 +3,7 @@ package dresources import ( "context" "fmt" + "slices" "strings" "github.com/databricks/cli/bundle/config/resources" @@ -48,14 +49,17 @@ func (r *ResourceVolume) DoCreate(ctx context.Context, config *catalog.CreateVol return response.FullName, response, nil } -func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, entry *PlanEntry) (*catalog.VolumeInfo, error) { +// See schemaForceSend. Verified against a real workspace: {"comment": ""} clears it. +var volumeForceSend = []string{"Comment"} + +func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, _ *PlanEntry) (*catalog.VolumeInfo, error) { updateRequest := catalog.UpdateVolumeRequestContent{ Comment: config.Comment, Name: id, NewName: "", // Not supported by Update(). Needs DoUpdateWithID() Owner: "", // Not supported by DABs - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below } nameFromID, err := getNameFromID(id) @@ -67,8 +71,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo return nil, fmt.Errorf("internal error: unexpected change of name from %#v to %#v", nameFromID, config.Name) } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(cleared, config.ForceSendFields...), "NewName", "Owner") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(slices.Clone(volumeForceSend), config.ForceSendFields...), "NewName", "Owner") response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil { @@ -82,7 +85,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo return response, err } -func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, entry *PlanEntry) (string, *catalog.VolumeInfo, error) { +func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateVolumeRequestContent, _ *PlanEntry) (string, *catalog.VolumeInfo, error) { updateRequest := catalog.UpdateVolumeRequestContent{ Comment: config.Comment, Name: id, @@ -90,7 +93,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * NewName: "", // Initialized below if needed Owner: "", // Not supported by DABs - ForceSendFields: nil, // set below, so the cleared fields go through the same exclusions + ForceSendFields: nil, // set below } items := strings.Split(id, ".") @@ -103,8 +106,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * updateRequest.NewName = config.Name } - cleared := forceSendClearedFields(&updateRequest, entry.Changes) - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(cleared, config.ForceSendFields...), "Owner") + updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(slices.Clone(volumeForceSend), config.ForceSendFields...), "Owner") response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil || response == nil { From 3a0a9644dd52b776961a6bb99a69a2eaa28751fe Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 25 Aug 2026 16:07:02 +0200 Subject: [PATCH 15/16] Drop the trailing blank line left in the schema test script Removing the retention section left the file ending in a blank line, which the whitespace linter fixes and CI then fails on. Co-authored-by: Isaac --- acceptance/bundle/resources/schemas/comment_out_of_band/script | 1 - 1 file changed, 1 deletion(-) diff --git a/acceptance/bundle/resources/schemas/comment_out_of_band/script b/acceptance/bundle/resources/schemas/comment_out_of_band/script index e0726bd56a2..29e2e1b8a2d 100644 --- a/acceptance/bundle/resources/schemas/comment_out_of_band/script +++ b/acceptance/bundle/resources/schemas/comment_out_of_band/script @@ -30,4 +30,3 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | title "Redeploy is a no-op" trace $CLI bundle deploy - From 35cab36da8e5ecba78376ed829f59fdb9bb36363 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 25 Aug 2026 16:45:07 +0200 Subject: [PATCH 16/16] Normalize the volume storage_location across clouds, and tidy the tests Recording the whole changes map surfaced the backend-generated storage_location, whose scheme and bucket differ per cloud, so the azure and gcp integration runs failed on an s3:// golden. Normalize to the AWS form ahead of the METASTORE_NAME repl, the same way grants/volumes does. Verified against azure and gcp. ForceSendFields is now assigned inline: the two-step only existed because the old reflection helper had to inspect the built request. print_requests.py moves out of the cleanup trap to just after the deploys, with the trap consuming the destroy's requests so none are left recorded. Co-authored-by: Isaac --- .../catalogs/comment_out_of_band/output.txt | 16 ++++----- .../catalogs/comment_out_of_band/script | 9 ++--- .../comment_cleared/output.txt | 16 ++++----- .../external_locations/comment_cleared/script | 8 +++-- .../comment_cleared/output.txt | 24 ++++++------- .../registered_models/comment_cleared/script | 8 +++-- .../schemas/comment_out_of_band/output.txt | 22 ++++++------ .../schemas/comment_out_of_band/script | 9 ++--- .../volumes/comment_out_of_band/output.txt | 34 +++++++++--------- .../volumes/comment_out_of_band/script | 9 ++--- .../volumes/comment_out_of_band/test.toml | 13 +++++++ .../volumes/comment_with_rename/output.txt | 36 +++++++++---------- .../volumes/comment_with_rename/script | 8 +++-- bundle/direct/dresources/catalog.go | 8 ++--- bundle/direct/dresources/external_location.go | 8 ++--- bundle/direct/dresources/registered_model.go | 4 +-- bundle/direct/dresources/schema.go | 4 +-- bundle/direct/dresources/volume.go | 8 ++--- 18 files changed, 125 insertions(+), 119 deletions(-) diff --git a/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt b/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt index ec723ea3a14..bc70893a2b5 100644 --- a/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt @@ -33,14 +33,6 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQU Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 1 unchanged ->>> [CLI] bundle destroy --auto-approve -The following resources will be deleted: - delete resources.catalogs.catalog1 - -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default - -Destroy: 1 deleted - === The UpdateCatalog payloads the deploys sent >>> print_requests.py --method PATCH //unity-catalog/catalogs { @@ -57,3 +49,11 @@ Destroy: 1 deleted "comment": "" } } + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.catalogs.catalog1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/resources/catalogs/comment_out_of_band/script b/acceptance/bundle/resources/catalogs/comment_out_of_band/script index dafb61ac5fb..fe406882746 100644 --- a/acceptance/bundle/resources/catalogs/comment_out_of_band/script +++ b/acceptance/bundle/resources/catalogs/comment_out_of_band/script @@ -3,13 +3,11 @@ envsubst < databricks.yml.tmpl > databricks.yml CATALOG="test-catalog-${UNIQUE_NAME}" -# print_requests.py consumes out.requests.txt, so it has to run after the last -# request the test makes, which is the destroy below. cleanup() { trace $CLI bundle destroy --auto-approve - title "The UpdateCatalog payloads the deploys sent" - trace print_requests.py --method PATCH //unity-catalog/catalogs + # Consume the destroy's requests so none are left recorded. + print_requests.py --method PATCH //unity-catalog/catalogs &> LOG.destroy_requests } trap cleanup EXIT @@ -30,3 +28,6 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/catalogs/$CATALOG" title "Redeploy is a no-op" trace $CLI bundle deploy + +title "The UpdateCatalog payloads the deploys sent" +trace print_requests.py --method PATCH //unity-catalog/catalogs diff --git a/acceptance/bundle/resources/external_locations/comment_cleared/output.txt b/acceptance/bundle/resources/external_locations/comment_cleared/output.txt index 0ba98552707..b005fd174fa 100644 --- a/acceptance/bundle/resources/external_locations/comment_cleared/output.txt +++ b/acceptance/bundle/resources/external_locations/comment_cleared/output.txt @@ -21,14 +21,6 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged >>> [CLI] bundle plan Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged ->>> [CLI] bundle destroy --auto-approve -The following resources will be deleted: - delete resources.external_locations.location1 - -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default - -Destroy: 1 deleted - === The UpdateExternalLocation payloads the deploys sent >>> print_requests.py --method PATCH //external-locations { @@ -40,3 +32,11 @@ Destroy: 1 deleted "url": "s3://test-bucket/path" } } + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.external_locations.location1 + +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/external_locations/comment_cleared/script b/acceptance/bundle/resources/external_locations/comment_cleared/script index f5a82dc9552..12317ae4a6b 100644 --- a/acceptance/bundle/resources/external_locations/comment_cleared/script +++ b/acceptance/bundle/resources/external_locations/comment_cleared/script @@ -1,11 +1,10 @@ echo "*" > .gitignore -# print_requests.py consumes out.requests.txt, so it runs after the last request. cleanup() { trace $CLI bundle destroy --auto-approve - title "The UpdateExternalLocation payloads the deploys sent" - trace print_requests.py --method PATCH //external-locations + # Consume the destroy's requests so none are left recorded. + print_requests.py --method PATCH //external-locations &> LOG.destroy_requests } trap cleanup EXIT @@ -21,3 +20,6 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/external-locations title "Redeploy" trace $CLI bundle plan + +title "The UpdateExternalLocation payloads the deploys sent" +trace print_requests.py --method PATCH //external-locations diff --git a/acceptance/bundle/resources/registered_models/comment_cleared/output.txt b/acceptance/bundle/resources/registered_models/comment_cleared/output.txt index eb5310e4653..44c63e3710c 100644 --- a/acceptance/bundle/resources/registered_models/comment_cleared/output.txt +++ b/acceptance/bundle/resources/registered_models/comment_cleared/output.txt @@ -22,18 +22,6 @@ Resources: 0 created, 1 changed, 0 deleted, 1 unchanged >>> [CLI] bundle plan Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged ->>> [CLI] bundle destroy --auto-approve -The following resources will be deleted: - delete resources.registered_models.model1 - delete resources.schemas.schema1 - -This action will result in the deletion of the following UC schemas. Any underlying data may be lost: - delete resources.schemas.schema1 - -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default - -Destroy: 2 deleted - === The UpdateRegisteredModel payloads the deploys sent >>> print_requests.py --method PATCH //unity-catalog/models { @@ -46,3 +34,15 @@ Destroy: 2 deleted "schema_name": "test-schema-[UNIQUE_NAME]" } } + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.registered_models.model1 + delete resources.schemas.schema1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 2 deleted diff --git a/acceptance/bundle/resources/registered_models/comment_cleared/script b/acceptance/bundle/resources/registered_models/comment_cleared/script index f466c7f6422..ad43f2474d2 100644 --- a/acceptance/bundle/resources/registered_models/comment_cleared/script +++ b/acceptance/bundle/resources/registered_models/comment_cleared/script @@ -3,12 +3,11 @@ envsubst < databricks.yml.tmpl > databricks.yml MODEL="main.test-schema-${UNIQUE_NAME}.test-model-${UNIQUE_NAME}" -# print_requests.py consumes out.requests.txt, so it runs after the last request. cleanup() { trace $CLI bundle destroy --auto-approve - title "The UpdateRegisteredModel payloads the deploys sent" - trace print_requests.py --method PATCH //unity-catalog/models + # Consume the destroy's requests so none are left recorded. + print_requests.py --method PATCH //unity-catalog/models &> LOG.destroy_requests } trap cleanup EXIT @@ -24,3 +23,6 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/models/$MODEL" | j title "Redeploy" trace $CLI bundle plan + +title "The UpdateRegisteredModel payloads the deploys sent" +trace print_requests.py --method PATCH //unity-catalog/models diff --git a/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt b/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt index b20e165a091..ea5395330f2 100644 --- a/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/schemas/comment_out_of_band/output.txt @@ -33,17 +33,6 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQU Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 1 unchanged ->>> [CLI] bundle destroy --auto-approve -The following resources will be deleted: - delete resources.schemas.schema1 - -This action will result in the deletion of the following UC schemas. Any underlying data may be lost: - delete resources.schemas.schema1 - -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default - -Destroy: 1 deleted - === The UpdateSchema payloads the deploys sent >>> print_requests.py --method PATCH //unity-catalog/schemas { @@ -60,3 +49,14 @@ Destroy: 1 deleted "comment": "" } } + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.schema1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/resources/schemas/comment_out_of_band/script b/acceptance/bundle/resources/schemas/comment_out_of_band/script index 29e2e1b8a2d..70871c43a9b 100644 --- a/acceptance/bundle/resources/schemas/comment_out_of_band/script +++ b/acceptance/bundle/resources/schemas/comment_out_of_band/script @@ -3,13 +3,11 @@ envsubst < databricks.yml.tmpl > databricks.yml SCHEMA="main.test-schema-${UNIQUE_NAME}" -# print_requests.py consumes out.requests.txt, so it has to run after the last -# request the test makes, which is the destroy below. cleanup() { trace $CLI bundle destroy --auto-approve - title "The UpdateSchema payloads the deploys sent" - trace print_requests.py --method PATCH //unity-catalog/schemas + # Consume the destroy's requests so none are left recorded. + print_requests.py --method PATCH //unity-catalog/schemas &> LOG.destroy_requests } trap cleanup EXIT @@ -30,3 +28,6 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/schemas/$SCHEMA" | title "Redeploy is a no-op" trace $CLI bundle deploy + +title "The UpdateSchema payloads the deploys sent" +trace print_requests.py --method PATCH //unity-catalog/schemas diff --git a/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt b/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt index a48c5501c24..a1b0f53281f 100644 --- a/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/volumes/comment_out_of_band/output.txt @@ -39,6 +39,23 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQU Files: 0 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 2 unchanged +=== The UpdateVolume payloads the deploys sent +>>> print_requests.py --method PATCH //unity-catalog/volumes +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].test-volume-[UNIQUE_NAME]", + "body": { + "comment": "set outside the bundle" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].test-volume-[UNIQUE_NAME]", + "body": { + "comment": "" + } +} + >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: delete resources.schemas.schema1 @@ -56,20 +73,3 @@ is removed from the catalog, but the underlying files are not deleted: All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default Destroy: 2 deleted - -=== The UpdateVolume payloads the deploys sent ->>> print_requests.py --method PATCH //unity-catalog/volumes -{ - "method": "PATCH", - "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].test-volume-[UNIQUE_NAME]", - "body": { - "comment": "set outside the bundle" - } -} -{ - "method": "PATCH", - "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].test-volume-[UNIQUE_NAME]", - "body": { - "comment": "" - } -} diff --git a/acceptance/bundle/resources/volumes/comment_out_of_band/script b/acceptance/bundle/resources/volumes/comment_out_of_band/script index 7b37d129aed..3c01424f369 100644 --- a/acceptance/bundle/resources/volumes/comment_out_of_band/script +++ b/acceptance/bundle/resources/volumes/comment_out_of_band/script @@ -3,13 +3,11 @@ envsubst < databricks.yml.tmpl > databricks.yml VOLUME="main.test-schema-${UNIQUE_NAME}.test-volume-${UNIQUE_NAME}" -# print_requests.py consumes out.requests.txt, so it has to run after the last -# request the test makes, which is the destroy below. cleanup() { trace $CLI bundle destroy --auto-approve - title "The UpdateVolume payloads the deploys sent" - trace print_requests.py --method PATCH //unity-catalog/volumes + # Consume the destroy's requests so none are left recorded. + print_requests.py --method PATCH //unity-catalog/volumes &> LOG.destroy_requests } trap cleanup EXIT @@ -30,3 +28,6 @@ MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/volumes/$VOLUME" | title "Redeploy is a no-op" trace $CLI bundle deploy + +title "The UpdateVolume payloads the deploys sent" +trace print_requests.py --method PATCH //unity-catalog/volumes diff --git a/acceptance/bundle/resources/volumes/comment_out_of_band/test.toml b/acceptance/bundle/resources/volumes/comment_out_of_band/test.toml index 27f8fb4be36..fa32d22b4e2 100644 --- a/acceptance/bundle/resources/volumes/comment_out_of_band/test.toml +++ b/acceptance/bundle/resources/volumes/comment_out_of_band/test.toml @@ -8,3 +8,16 @@ Ignore = [ ".gitignore", "databricks.yml", ] + +# The plan records the backend-generated storage_location, whose scheme and bucket differ +# per cloud. Normalize to the AWS form before the METASTORE_NAME repl, same as +# grants/volumes/test.toml. +[[Repls]] +Old = 'abfss://decotestprod-unity-iso@decotestprodunityiso.dfs.core.windows.net' +New = 's3://deco-uc-prod-isolated-aws-us-east-1/metastore' +Order = -1 + +[[Repls]] +Old = 'gs://deco-uc-prod-gcp-us-central1-isolated' +New = 's3://deco-uc-prod-isolated-aws-us-east-1' +Order = -1 diff --git a/acceptance/bundle/resources/volumes/comment_with_rename/output.txt b/acceptance/bundle/resources/volumes/comment_with_rename/output.txt index 3713e45f084..953800978e3 100644 --- a/acceptance/bundle/resources/volumes/comment_with_rename/output.txt +++ b/acceptance/bundle/resources/volumes/comment_with_rename/output.txt @@ -21,24 +21,6 @@ Resources: 0 created, 1 changed, 0 deleted, 1 unchanged >>> [CLI] bundle plan Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged ->>> [CLI] bundle destroy --auto-approve -The following resources will be deleted: - delete resources.schemas.schema1 - delete resources.volumes.volume1 - -This action will result in the deletion of the following UC schemas. Any underlying data may be lost: - delete resources.schemas.schema1 - -This action will result in the deletion of the following volumes. -For managed volumes, the files stored in the volume are also deleted from your -cloud tenant within 30 days. For external volumes, the metadata about the volume -is removed from the catalog, but the underlying files are not deleted: - delete resources.volumes.volume1 - -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default - -Destroy: 2 deleted - === The rename carried the cleared comment alongside new_name >>> print_requests.py --method PATCH //unity-catalog/volumes { @@ -56,3 +38,21 @@ Destroy: 2 deleted "new_name": "vol_two" } } + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.schema1 + delete resources.volumes.volume1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +This action will result in the deletion of the following volumes. +For managed volumes, the files stored in the volume are also deleted from your +cloud tenant within 30 days. For external volumes, the metadata about the volume +is removed from the catalog, but the underlying files are not deleted: + delete resources.volumes.volume1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 2 deleted diff --git a/acceptance/bundle/resources/volumes/comment_with_rename/script b/acceptance/bundle/resources/volumes/comment_with_rename/script index 5229e4ef5c8..2a9cce62ec5 100644 --- a/acceptance/bundle/resources/volumes/comment_with_rename/script +++ b/acceptance/bundle/resources/volumes/comment_with_rename/script @@ -3,12 +3,11 @@ envsubst < databricks.yml.tmpl > databricks.yml VOLUME="main.test-schema-${UNIQUE_NAME}.vol_one" -# print_requests.py consumes out.requests.txt, so it runs after the last request. cleanup() { trace $CLI bundle destroy --auto-approve - title "The rename carried the cleared comment alongside new_name" - trace print_requests.py --method PATCH //unity-catalog/volumes + # Consume the destroy's requests so none are left recorded. + print_requests.py --method PATCH //unity-catalog/volumes &> LOG.destroy_requests } trap cleanup EXIT @@ -23,3 +22,6 @@ trace $CLI bundle deploy title "The rename and the clear both landed, so it converges in one pass" MSYS_NO_PATHCONV=1 trace $CLI api get "/api/2.1/unity-catalog/volumes/main.test-schema-${UNIQUE_NAME}.vol_two" | jq '.comment' trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged" + +title "The rename carried the cleared comment alongside new_name" +trace print_requests.py --method PATCH //unity-catalog/volumes diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 17b36a2e069..b29f1fa90c6 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -66,11 +66,9 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](append(slices.Clone(catalogForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner"), } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(slices.Clone(catalogForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") - response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { return nil, err @@ -92,15 +90,13 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config Options: config.Options, Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](append(slices.Clone(catalogForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner"), } if config.Name != id { updateRequest.NewName = config.Name } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateCatalog](append(slices.Clone(catalogForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "IsolationMode", "Owner") - response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { return "", nil, err diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index fedcb606ecb..26c957020ab 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -79,11 +79,9 @@ func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, conf ReadOnly: config.ReadOnly, SkipValidation: config.SkipValidation, Url: config.Url, - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](append(slices.Clone(externalLocationForceSend), config.ForceSendFields...), "IsolationMode", "Owner"), } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(slices.Clone(externalLocationForceSend), config.ForceSendFields...), "IsolationMode", "Owner") - return r.client.ExternalLocations.Update(ctx, updateRequest) } @@ -106,15 +104,13 @@ func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string ReadOnly: config.ReadOnly, SkipValidation: config.SkipValidation, Url: config.Url, - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](append(slices.Clone(externalLocationForceSend), config.ForceSendFields...), "IsolationMode", "Owner"), } if config.Name != id { updateRequest.NewName = config.Name } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateExternalLocation](append(slices.Clone(externalLocationForceSend), config.ForceSendFields...), "IsolationMode", "Owner") - response, err := r.client.ExternalLocations.Update(ctx, updateRequest) if err != nil { return "", nil, err diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index f3d708ff786..e3c67b3d8cf 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -73,7 +73,7 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi updateRequest := catalog.UpdateRegisteredModelRequest{ FullName: id, Comment: config.Comment, - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateRegisteredModelRequest](append(slices.Clone(registeredModelForceSend), config.ForceSendFields...), "Owner", "NewName"), // Owner is settable in the config (it comes from the embedded // CreateRegisteredModelRequest) and create sends it, but update never has: a @@ -97,8 +97,6 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi CatalogName: config.CatalogName, } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateRegisteredModelRequest](append(slices.Clone(registeredModelForceSend), config.ForceSendFields...), "Owner", "NewName") - response, err := r.client.RegisteredModels.Update(ctx, updateRequest) if err != nil { return nil, err diff --git a/bundle/direct/dresources/schema.go b/bundle/direct/dresources/schema.go index 872b2b26dc4..b29c89bdcee 100644 --- a/bundle/direct/dresources/schema.go +++ b/bundle/direct/dresources/schema.go @@ -73,11 +73,9 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // We recreate schemas on name change intentionally. Owner: "", // Not supported by DABs Properties: config.Properties, - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateSchema](append(slices.Clone(schemaForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "NewName", "Owner"), } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateSchema](append(slices.Clone(schemaForceSend), config.ForceSendFields...), "EnablePredictiveOptimization", "NewName", "Owner") - response, err := r.client.Schemas.Update(ctx, updateRequest) if err != nil { return nil, err diff --git a/bundle/direct/dresources/volume.go b/bundle/direct/dresources/volume.go index 2c6967a7998..7cd1eaba1ce 100644 --- a/bundle/direct/dresources/volume.go +++ b/bundle/direct/dresources/volume.go @@ -59,7 +59,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo NewName: "", // Not supported by Update(). Needs DoUpdateWithID() Owner: "", // Not supported by DABs - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](append(slices.Clone(volumeForceSend), config.ForceSendFields...), "NewName", "Owner"), } nameFromID, err := getNameFromID(id) @@ -71,8 +71,6 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo return nil, fmt.Errorf("internal error: unexpected change of name from %#v to %#v", nameFromID, config.Name) } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(slices.Clone(volumeForceSend), config.ForceSendFields...), "NewName", "Owner") - response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil { return nil, err @@ -93,7 +91,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * NewName: "", // Initialized below if needed Owner: "", // Not supported by DABs - ForceSendFields: nil, // set below + ForceSendFields: utils.FilterFields[catalog.UpdateVolumeRequestContent](append(slices.Clone(volumeForceSend), config.ForceSendFields...), "Owner"), } items := strings.Split(id, ".") @@ -106,8 +104,6 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config * updateRequest.NewName = config.Name } - updateRequest.ForceSendFields = utils.FilterFields[catalog.UpdateVolumeRequestContent](append(slices.Clone(volumeForceSend), config.ForceSendFields...), "Owner") - response, err := r.client.Volumes.Update(ctx, updateRequest) if err != nil || response == nil { return "", nil, err