diff --git a/.nextchanges/bundles/catalog-renamed-out-of-band.md b/.nextchanges/bundles/catalog-renamed-out-of-band.md new file mode 100644 index 00000000000..9ca1bd630c8 --- /dev/null +++ b/.nextchanges/bundles/catalog-renamed-out-of-band.md @@ -0,0 +1 @@ +* A catalog renamed outside the bundle is now found by its id, instead of being created again (which failed with `CATALOG_ALREADY_EXISTS`). ([#7418](https://github.com/databricks/cli/pull/7418)) diff --git a/acceptance/bundle/refschema/output/catalogs.txt b/acceptance/bundle/refschema/output/catalogs.txt index feaee00d2ca..abd7685dc55 100644 --- a/acceptance/bundle/refschema/output/catalogs.txt +++ b/acceptance/bundle/refschema/output/catalogs.txt @@ -11,7 +11,7 @@ resources.catalogs.*.effective_predictive_optimization_flag.inherited_from_type resources.catalogs.*.effective_predictive_optimization_flag.value catalog.EnablePredictiveOptimization REMOTE resources.catalogs.*.enable_predictive_optimization catalog.EnablePredictiveOptimization REMOTE resources.catalogs.*.full_name string REMOTE -resources.catalogs.*.id string INPUT +resources.catalogs.*.id string ALL resources.catalogs.*.isolation_mode catalog.CatalogIsolationMode REMOTE resources.catalogs.*.lifecycle resources.Lifecycle INPUT resources.catalogs.*.lifecycle.depends_on []resources.DependsOn INPUT 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 58bebd5aac2..6d8e4cee373 100644 --- a/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt +++ b/acceptance/bundle/resources/catalogs/comment_out_of_band/output.txt @@ -13,6 +13,12 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged "comment": { "action": "update", "remote": "set outside the bundle" + }, + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" } } diff --git a/acceptance/bundle/resources/catalogs/drift/managed_properties/output.txt b/acceptance/bundle/resources/catalogs/drift/managed_properties/output.txt index 5c22507bb39..f3a71b13b37 100644 --- a/acceptance/bundle/resources/catalogs/drift/managed_properties/output.txt +++ b/acceptance/bundle/resources/catalogs/drift/managed_properties/output.txt @@ -13,6 +13,12 @@ Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged === The remote-only properties map is skipped as a backend default (confirms the matched rule) >>> [CLI] bundle plan --output json { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + }, "properties": { "action": "skip", "reason": "backend_default", diff --git a/acceptance/bundle/resources/catalogs/rename/output.txt b/acceptance/bundle/resources/catalogs/rename/output.txt index e07288217d4..14acf8e62e9 100644 --- a/acceptance/bundle/resources/catalogs/rename/output.txt +++ b/acceptance/bundle/resources/catalogs/rename/output.txt @@ -53,10 +53,17 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged "created_at": [UNIX_TIME_MILLIS][0], "created_by": "[USERNAME]", "full_name": "test_catalog_[UNIQUE_NAME]", + "id": "[UUID]", "name": "test_catalog_[UNIQUE_NAME]", "owner": "[USERNAME]" }, "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + }, "name": { "action": "update_id", "reason": "id_changes", diff --git a/acceptance/bundle/resources/catalogs/rename_all_out_of_band/output.txt b/acceptance/bundle/resources/catalogs/rename_all_out_of_band/output.txt index bd744dd4686..ecf793c2d23 100644 --- a/acceptance/bundle/resources/catalogs/rename_all_out_of_band/output.txt +++ b/acceptance/bundle/resources/catalogs/rename_all_out_of_band/output.txt @@ -31,22 +31,40 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged >>> update_file.py databricks.yml name: myvolume name: myvolume2 -=== The plan finds the schema and the volume at their new names by their ids. A catalog has no such id, so it is created again +=== The plan finds the catalog, the schema and the volume at their new names by their ids >>> [CLI] bundle plan --output json { "resources.catalogs.catalog1": { - "action": "create", + "id": "test_catalog_renamed_[UNIQUE_NAME]", + "action": "update_id", + "state_only": true, "new_state": { "value": { "name": "test_catalog_renamed_[UNIQUE_NAME]" } }, + "remote_state": { + "catalog_type": "MANAGED_CATALOG", + "created_at": [UNIX_TIME_MILLIS][0], + "created_by": "[USERNAME]", + "full_name": "test_catalog_renamed_[UNIQUE_NAME]", + "id": "[CATALOG_ID]", + "name": "test_catalog_renamed_[UNIQUE_NAME]", + "owner": "[USERNAME]" + }, "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[CATALOG_ID]", + "remote": "[CATALOG_ID]" + }, "name": { - "action": "update_id", - "reason": "id_changes", + "action": "skip", + "reason": "remote_already_set", "old": "test_catalog_[UNIQUE_NAME]", - "new": "test_catalog_renamed_[UNIQUE_NAME]" + "new": "test_catalog_renamed_[UNIQUE_NAME]", + "remote": "test_catalog_renamed_[UNIQUE_NAME]" } } }, @@ -69,7 +87,7 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged "remote_state": { "catalog_name": "test_catalog_renamed_[UNIQUE_NAME]", "catalog_type": "MANAGED_CATALOG", - "created_at": [UNIX_TIME_MILLIS][0], + "created_at": [UNIX_TIME_MILLIS][1], "created_by": "[USERNAME]", "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema2", "name": "myschema2", @@ -123,7 +141,7 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged }, "remote_state": { "catalog_name": "test_catalog_renamed_[UNIQUE_NAME]", - "created_at": [UNIX_TIME_MILLIS][1], + "created_at": [UNIX_TIME_MILLIS][2], "created_by": "[USERNAME]", "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema2.myvolume2", "name": "myvolume2", @@ -164,25 +182,46 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged } } -=== The catalog's create fails because the renamed catalog has its name ->>> musterr [CLI] bundle deploy +=== Deploy only records the new names in the state +>>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... -Error: cannot create resources.catalogs.catalog1: Catalog 'test_catalog_renamed_[UNIQUE_NAME]' already exists (400 CATALOG_ALREADY_EXISTS) +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 3 unchanged + +>>> print_requests.py //unity-catalog + +>>> [CLI] bundle summary --output json +{ + "catalogs": { + "catalog1": "test_catalog_renamed_[UNIQUE_NAME]" + }, + "schemas": { + "schema1": "test_catalog_renamed_[UNIQUE_NAME].myschema2" + }, + "volumes": { + "volume1": "test_catalog_renamed_[UNIQUE_NAME].myschema2.myvolume2" + } +} -Endpoint: POST [DATABRICKS_URL]/api/2.1/unity-catalog/catalogs -HTTP Status: 400 Bad Request -API error_code: CATALOG_ALREADY_EXISTS -API message: Catalog 'test_catalog_renamed_[UNIQUE_NAME]' already exists +=== Redeploy is a no-op +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 3 unchanged -Error: cannot update_id resources.schemas.schema1: dependency failed: resources.catalogs.catalog1 +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.catalogs.catalog1 + delete resources.schemas.schema1 + delete resources.volumes.volume1 -Error: cannot update_id resources.volumes.volume1: dependency failed: resources.catalogs.catalog1 +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 -Files: 3 uploaded, 0 deleted +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 ->>> errcode [CLI] bundle destroy --auto-approve All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default -Destroy: 0 deleted - ->>> errcode [CLI] catalogs delete test_catalog_renamed_[UNIQUE_NAME] --force +Destroy: 3 deleted diff --git a/acceptance/bundle/resources/catalogs/rename_all_out_of_band/script b/acceptance/bundle/resources/catalogs/rename_all_out_of_band/script index 9645fc0dc02..26fed081e3c 100644 --- a/acceptance/bundle/resources/catalogs/rename_all_out_of_band/script +++ b/acceptance/bundle/resources/catalogs/rename_all_out_of_band/script @@ -4,9 +4,7 @@ OLD_CATALOG="test_catalog_${UNIQUE_NAME}" NEW_CATALOG="test_catalog_renamed_${UNIQUE_NAME}" cleanup() { - trace errcode $CLI bundle destroy --auto-approve - # The renamed catalog is not where the state looks for it, so destroy cannot remove it. - trace errcode $CLI catalogs delete "$NEW_CATALOG" --force + trace $CLI bundle destroy --auto-approve # Consume the destroy's requests so none are left recorded. print_requests.py //unity-catalog &> LOG.destroy_requests @@ -21,6 +19,8 @@ plan() { title "Deploy a catalog with a schema and a volume in it" trace $CLI bundle deploy print_requests.py //unity-catalog &> LOG.deploy_requests +# The SDK drops the catalog's id, so read it with a raw GET. +add_repl "$(MSYS_NO_PATHCONV=1 $CLI api get "/api/2.1/unity-catalog/catalogs/$OLD_CATALOG" | jq -r .id)" CATALOG_ID add_repl "$($CLI schemas get "$OLD_CATALOG.myschema" | jq -r .schema_id)" SCHEMA_ID add_repl "$($CLI volumes read "$OLD_CATALOG.myschema.myvolume" | jq -r .volume_id)" VOLUME_ID @@ -34,11 +34,14 @@ title "Rename all three in the config to match" trace update_file.py databricks.yml test_catalog_ test_catalog_renamed_ trace update_file.py databricks.yml "name: myschema" "name: myschema2" trace update_file.py databricks.yml "name: myvolume" "name: myvolume2" -NEW_SCHEMA="$NEW_CATALOG.myschema2" -NEW_VOLUME="$NEW_SCHEMA.myvolume2" -title "The plan finds the schema and the volume at their new names by their ids. A catalog has no such id, so it is created again" +title "The plan finds the catalog, the schema and the volume at their new names by their ids" plan -title "The catalog's create fails because the renamed catalog has its name" -trace musterr $CLI bundle deploy +title "Deploy only records the new names in the state" +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace $CLI bundle summary --output json | jq '.resources | map_values(map_values(.id))' + +title "Redeploy is a no-op" +trace $CLI bundle plan diff --git a/acceptance/bundle/resources/catalogs/rename_all_out_of_band/test.toml b/acceptance/bundle/resources/catalogs/rename_all_out_of_band/test.toml index 698c1c6ca49..4272a80cf61 100644 --- a/acceptance/bundle/resources/catalogs/rename_all_out_of_band/test.toml +++ b/acceptance/bundle/resources/catalogs/rename_all_out_of_band/test.toml @@ -1,4 +1,3 @@ -Badness = "The plan does not look for a catalog renamed outside the bundle, so it creates it again: the create fails with CATALOG_ALREADY_EXISTS on every deploy, and destroy leaves the renamed catalog behind. Same on main." Cloud = true Ignore = [ diff --git a/acceptance/bundle/resources/catalogs/rename_out_of_band/output.txt b/acceptance/bundle/resources/catalogs/rename_out_of_band/output.txt index 350b55d2a5e..1269b63a57b 100644 --- a/acceptance/bundle/resources/catalogs/rename_out_of_band/output.txt +++ b/acceptance/bundle/resources/catalogs/rename_out_of_band/output.txt @@ -17,22 +17,40 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged === Rename the catalog in the config to match >>> update_file.py databricks.yml test_catalog_ test_catalog_renamed_ -=== The plan finds the schema and the volume at their new names by their ids. A catalog has no such id, so it is created again +=== The plan finds the catalog, the schema and the volume at their new names by their ids >>> [CLI] bundle plan --output json { "resources.catalogs.catalog1": { - "action": "create", + "id": "test_catalog_renamed_[UNIQUE_NAME]", + "action": "update_id", + "state_only": true, "new_state": { "value": { "name": "test_catalog_renamed_[UNIQUE_NAME]" } }, + "remote_state": { + "catalog_type": "MANAGED_CATALOG", + "created_at": [UNIX_TIME_MILLIS][0], + "created_by": "[USERNAME]", + "full_name": "test_catalog_renamed_[UNIQUE_NAME]", + "id": "[CATALOG_ID]", + "name": "test_catalog_renamed_[UNIQUE_NAME]", + "owner": "[USERNAME]" + }, "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[CATALOG_ID]", + "remote": "[CATALOG_ID]" + }, "name": { - "action": "update_id", - "reason": "id_changes", + "action": "skip", + "reason": "remote_already_set", "old": "test_catalog_[UNIQUE_NAME]", - "new": "test_catalog_renamed_[UNIQUE_NAME]" + "new": "test_catalog_renamed_[UNIQUE_NAME]", + "remote": "test_catalog_renamed_[UNIQUE_NAME]" } } }, @@ -55,7 +73,7 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged "remote_state": { "catalog_name": "test_catalog_renamed_[UNIQUE_NAME]", "catalog_type": "MANAGED_CATALOG", - "created_at": [UNIX_TIME_MILLIS][0], + "created_at": [UNIX_TIME_MILLIS][1], "created_by": "[USERNAME]", "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema", "name": "myschema", @@ -102,7 +120,7 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged }, "remote_state": { "catalog_name": "test_catalog_renamed_[UNIQUE_NAME]", - "created_at": [UNIX_TIME_MILLIS][1], + "created_at": [UNIX_TIME_MILLIS][2], "created_by": "[USERNAME]", "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema.myvolume", "name": "myvolume", @@ -129,25 +147,46 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged } } -=== The catalog's create fails because the renamed catalog has its name ->>> musterr [CLI] bundle deploy +=== Deploy only records the new names in the state +>>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... -Error: cannot create resources.catalogs.catalog1: Catalog 'test_catalog_renamed_[UNIQUE_NAME]' already exists (400 CATALOG_ALREADY_EXISTS) +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 3 unchanged + +>>> print_requests.py //unity-catalog + +>>> [CLI] bundle summary --output json +{ + "catalogs": { + "catalog1": "test_catalog_renamed_[UNIQUE_NAME]" + }, + "schemas": { + "schema1": "test_catalog_renamed_[UNIQUE_NAME].myschema" + }, + "volumes": { + "volume1": "test_catalog_renamed_[UNIQUE_NAME].myschema.myvolume" + } +} -Endpoint: POST [DATABRICKS_URL]/api/2.1/unity-catalog/catalogs -HTTP Status: 400 Bad Request -API error_code: CATALOG_ALREADY_EXISTS -API message: Catalog 'test_catalog_renamed_[UNIQUE_NAME]' already exists +=== Redeploy is a no-op +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 3 unchanged -Error: cannot update_id resources.schemas.schema1: dependency failed: resources.catalogs.catalog1 +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.catalogs.catalog1 + delete resources.schemas.schema1 + delete resources.volumes.volume1 -Error: cannot update_id resources.volumes.volume1: dependency failed: resources.catalogs.catalog1 +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 -Files: 3 uploaded, 0 deleted +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 ->>> errcode [CLI] bundle destroy --auto-approve All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default -Destroy: 0 deleted - ->>> errcode [CLI] catalogs delete test_catalog_renamed_[UNIQUE_NAME] --force +Destroy: 3 deleted diff --git a/acceptance/bundle/resources/catalogs/rename_out_of_band/script b/acceptance/bundle/resources/catalogs/rename_out_of_band/script index 8b161d76a37..d94cb0278ff 100644 --- a/acceptance/bundle/resources/catalogs/rename_out_of_band/script +++ b/acceptance/bundle/resources/catalogs/rename_out_of_band/script @@ -4,9 +4,7 @@ OLD_CATALOG="test_catalog_${UNIQUE_NAME}" NEW_CATALOG="test_catalog_renamed_${UNIQUE_NAME}" cleanup() { - trace errcode $CLI bundle destroy --auto-approve - # The renamed catalog is not where the state looks for it, so destroy cannot remove it. - trace errcode $CLI catalogs delete "$NEW_CATALOG" --force + trace $CLI bundle destroy --auto-approve # Consume the destroy's requests so none are left recorded. print_requests.py //unity-catalog &> LOG.destroy_requests @@ -21,6 +19,8 @@ plan() { title "Deploy a catalog with a schema and a volume in it" trace $CLI bundle deploy print_requests.py //unity-catalog &> LOG.deploy_requests +# The SDK drops the catalog's id, so read it with a raw GET. +add_repl "$(MSYS_NO_PATHCONV=1 $CLI api get "/api/2.1/unity-catalog/catalogs/$OLD_CATALOG" | jq -r .id)" CATALOG_ID add_repl "$($CLI schemas get "$OLD_CATALOG.myschema" | jq -r .schema_id)" SCHEMA_ID add_repl "$($CLI volumes read "$OLD_CATALOG.myschema.myvolume" | jq -r .volume_id)" VOLUME_ID @@ -30,11 +30,14 @@ print_requests.py //unity-catalog &> LOG.rename_requests title "Rename the catalog in the config to match" trace update_file.py databricks.yml test_catalog_ test_catalog_renamed_ -NEW_SCHEMA="$NEW_CATALOG.myschema" -NEW_VOLUME="$NEW_SCHEMA.myvolume" -title "The plan finds the schema and the volume at their new names by their ids. A catalog has no such id, so it is created again" +title "The plan finds the catalog, the schema and the volume at their new names by their ids" plan -title "The catalog's create fails because the renamed catalog has its name" -trace musterr $CLI bundle deploy +title "Deploy only records the new names in the state" +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace $CLI bundle summary --output json | jq '.resources | map_values(map_values(.id))' + +title "Redeploy is a no-op" +trace $CLI bundle plan diff --git a/acceptance/bundle/resources/catalogs/rename_out_of_band/test.toml b/acceptance/bundle/resources/catalogs/rename_out_of_band/test.toml index 698c1c6ca49..4272a80cf61 100644 --- a/acceptance/bundle/resources/catalogs/rename_out_of_band/test.toml +++ b/acceptance/bundle/resources/catalogs/rename_out_of_band/test.toml @@ -1,4 +1,3 @@ -Badness = "The plan does not look for a catalog renamed outside the bundle, so it creates it again: the create fails with CATALOG_ALREADY_EXISTS on every deploy, and destroy leaves the renamed catalog behind. Same on main." Cloud = true Ignore = [ diff --git a/acceptance/bundle/resources/grants/catalogs/out.plan2.json b/acceptance/bundle/resources/grants/catalogs/out.plan2.json index 92d227bc232..9850835017c 100644 --- a/acceptance/bundle/resources/grants/catalogs/out.plan2.json +++ b/acceptance/bundle/resources/grants/catalogs/out.plan2.json @@ -10,8 +10,17 @@ "catalog_type": "MANAGED_CATALOG", "comment": "Test catalog for grants", "full_name": "catalog_grants_[UNIQUE_NAME]", + "id": "[UUID]", "name": "catalog_grants_[UNIQUE_NAME]", "owner": "[USERNAME]" + }, + "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + } } }, "resources.catalogs.grants_catalog.grants": { diff --git a/acceptance/bundle/resources/registered_models/rename/output.txt b/acceptance/bundle/resources/registered_models/rename/output.txt index a85544f2483..a89cbf29f6d 100644 --- a/acceptance/bundle/resources/registered_models/rename/output.txt +++ b/acceptance/bundle/resources/registered_models/rename/output.txt @@ -46,8 +46,17 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged "catalog_type": "MANAGED_CATALOG", "created_by": "[USERNAME]", "full_name": "test_catalog_[UNIQUE_NAME]", + "id": "[UUID]", "name": "test_catalog_[UNIQUE_NAME]", "owner": "[USERNAME]" + }, + "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + } } }, "resources.registered_models.model1": { @@ -167,8 +176,17 @@ Resources: 0 created, 1 changed, 0 deleted, 2 unchanged "catalog_type": "MANAGED_CATALOG", "created_by": "[USERNAME]", "full_name": "test_catalog_[UNIQUE_NAME]", + "id": "[UUID]", "name": "test_catalog_[UNIQUE_NAME]", "owner": "[USERNAME]" + }, + "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + } } }, "resources.registered_models.model1": { @@ -306,10 +324,17 @@ Resources: 0 created, 1 changed, 0 deleted, 2 unchanged "catalog_type": "MANAGED_CATALOG", "created_by": "[USERNAME]", "full_name": "test_catalog_[UNIQUE_NAME]", + "id": "[UUID]", "name": "test_catalog_[UNIQUE_NAME]", "owner": "[USERNAME]" }, "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + }, "name": { "action": "update_id", "reason": "id_changes", diff --git a/acceptance/bundle/resources/secrets/parent_rename/output.txt b/acceptance/bundle/resources/secrets/parent_rename/output.txt index 929a3b532dd..5e5ab244fd6 100644 --- a/acceptance/bundle/resources/secrets/parent_rename/output.txt +++ b/acceptance/bundle/resources/secrets/parent_rename/output.txt @@ -47,8 +47,17 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged "catalog_type": "MANAGED_CATALOG", "created_by": "[USERNAME]", "full_name": "test_catalog_[UNIQUE_NAME]", + "id": "[UUID]", "name": "test_catalog_[UNIQUE_NAME]", "owner": "[USERNAME]" + }, + "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + } } }, "resources.schemas.schema1": { @@ -222,10 +231,17 @@ Resources: 0 created, 1 changed, 0 deleted, 2 unchanged "catalog_type": "MANAGED_CATALOG", "created_by": "[USERNAME]", "full_name": "test_catalog_[UNIQUE_NAME]", + "id": "[UUID]", "name": "test_catalog_[UNIQUE_NAME]", "owner": "[USERNAME]" }, "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + }, "name": { "action": "update_id", "reason": "id_changes", diff --git a/acceptance/bundle/resources/volumes/rename_catalog_and_schema/output.txt b/acceptance/bundle/resources/volumes/rename_catalog_and_schema/output.txt index e8fd1ecfdde..37d83d27b58 100644 --- a/acceptance/bundle/resources/volumes/rename_catalog_and_schema/output.txt +++ b/acceptance/bundle/resources/volumes/rename_catalog_and_schema/output.txt @@ -28,10 +28,17 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged "created_at": [UNIX_TIME_MILLIS][0], "created_by": "[USERNAME]", "full_name": "test_catalog_[UNIQUE_NAME]", + "id": "[UUID]", "name": "test_catalog_[UNIQUE_NAME]", "owner": "[USERNAME]" }, "changes": { + "id": { + "action": "skip", + "reason": "internal_id", + "old": "[UUID]", + "remote": "[UUID]" + }, "name": { "action": "update_id", "reason": "id_changes", diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 638138bfbda..866876b730d 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -2,28 +2,101 @@ package dresources import ( "context" + "maps" + "net/http" "slices" + "sync" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/auth" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/client" + "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" ) +const catalogsPath = "/api/2.1/unity-catalog/catalogs" + +// CatalogRemote is CatalogInfo plus the catalog's id, a UUID that UC returns but the API spec does +// not declare, so the SDK drops it. A rename keeps the id and a recreate changes it (verified on +// AWS, Azure and GCP). +type CatalogRemote struct { + catalog.CatalogInfo + + Id string `json:"id,omitempty"` +} + +// Custom marshaler needed because embedded CatalogInfo has its own MarshalJSON +// which would otherwise take over and ignore the additional fields. +func (s *CatalogRemote) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, s) +} + +func (s CatalogRemote) MarshalJSON() ([]byte, error) { + return marshal.Marshal(s) +} + +type CatalogState struct { + catalog.CreateCatalog + + // Recorded by the framework from the remote state (internal_id_field). + Id string `json:"id,omitempty"` +} + +// Custom marshaler needed because embedded CreateCatalog has its own MarshalJSON +// which would otherwise take over and ignore the additional fields. +func (s *CatalogState) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, s) +} + +func (s CatalogState) MarshalJSON() ([]byte, error) { + return marshal.Marshal(s) +} + type ResourceCatalog struct { - client *databricks.WorkspaceClient + client *databricks.WorkspaceClient + apiClient func() (*client.DatabricksClient, error) } -func (*ResourceCatalog) New(client *databricks.WorkspaceClient) *ResourceCatalog { - return &ResourceCatalog{client: client} +func (*ResourceCatalog) New(w *databricks.WorkspaceClient) *ResourceCatalog { + return &ResourceCatalog{ + client: w, + apiClient: sync.OnceValues(func() (*client.DatabricksClient, error) { + return client.New(w.Config) + }), + } } -func (*ResourceCatalog) PrepareState(input *resources.Catalog) *catalog.CreateCatalog { - return &input.CreateCatalog +// do calls the catalogs API like the SDK does, but decodes the response into CatalogRemote: +// the SDK's CatalogInfo has no id. +func (r *ResourceCatalog) do(ctx context.Context, method, path string, request any) (*CatalogRemote, error) { + apiClient, err := r.apiClient() + if err != nil { + return nil, err + } + headers := map[string]string{"Accept": "application/json"} + if request != nil { + headers["Content-Type"] = "application/json" + } + maps.Copy(headers, auth.WorkspaceIDHeaders(r.client.Config)) + var response CatalogRemote + err = apiClient.Do(ctx, method, path, headers, nil, request, &response) + if err != nil { + return nil, err + } + return &response, nil +} + +func (*ResourceCatalog) PrepareState(input *resources.Catalog) *CatalogState { + return &CatalogState{ + CreateCatalog: input.CreateCatalog, + Id: "", // Recorded by the framework from the remote state (internal_id_field). + } } -func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCatalog { - return &catalog.CreateCatalog{ +func (*ResourceCatalog) RemapState(info *CatalogRemote) *CatalogState { + return &CatalogState{ Comment: info.Comment, ConnectionName: info.ConnectionName, CustomMaxRetentionHours: info.CustomMaxRetentionHours, @@ -35,21 +108,27 @@ func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCat ShareName: info.ShareName, StorageRoot: info.StorageRoot, ForceSendFields: utils.FilterFields[catalog.CreateCatalog](info.ForceSendFields), + Id: info.Id, } } -func (r *ResourceCatalog) DoRead(ctx context.Context, id string) (*catalog.CatalogInfo, error) { - return r.client.Catalogs.GetByName(ctx, id) +func (r *ResourceCatalog) DoRead(ctx context.Context, id string) (*CatalogRemote, error) { + return r.do(ctx, http.MethodGet, catalogsPath+"/"+id, nil) } -func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCatalog) (string, *catalog.CatalogInfo, error) { - response, err := r.client.Catalogs.Create(ctx, *config) - if err != nil || response == nil { +func (r *ResourceCatalog) DoCreate(ctx context.Context, config *CatalogState) (string, *CatalogRemote, error) { + response, err := r.do(ctx, http.MethodPost, catalogsPath, config.CreateCatalog) + if err != nil { return "", nil, err } return response.Name, response, nil } +// IDFromState returns the name, which is the ID. +func (*ResourceCatalog) IDFromState(state *CatalogState) string { + return state.Name +} + // See schemaForceSend. Verified against a real workspace: {"comment": ""} clears it. // // CustomMaxRetentionHours is force-sent here even though schemaForceSend deliberately omits it: @@ -59,7 +138,7 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa var catalogForceSend = []string{"Comment", "CustomMaxRetentionHours"} // 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 *CatalogState, _ *PlanEntry) (*CatalogRemote, error) { updateRequest := catalog.UpdateCatalog{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -74,16 +153,11 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal 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 - } - - return response, nil + return r.do(ctx, http.MethodPatch, catalogsPath+"/"+id, updateRequest) } // 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 *CatalogState, _ *PlanEntry) (string, *CatalogRemote, error) { updateRequest := catalog.UpdateCatalog{ Comment: config.Comment, CustomMaxRetentionHours: config.CustomMaxRetentionHours, @@ -102,7 +176,7 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config updateRequest.NewName = config.Name } - response, err := r.client.Catalogs.Update(ctx, updateRequest) + response, err := r.do(ctx, http.MethodPatch, catalogsPath+"/"+id, updateRequest) if err != nil { return "", nil, err } @@ -116,7 +190,7 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config return newID, response, nil } -func (r *ResourceCatalog) DoDelete(ctx context.Context, id string, _ *catalog.CreateCatalog) error { +func (r *ResourceCatalog) DoDelete(ctx context.Context, id string, _ *CatalogState) error { return r.client.Catalogs.Delete(ctx, catalog.DeleteCatalogRequest{ Name: id, Force: true, diff --git a/bundle/direct/dresources/configs/catalogs.yml b/bundle/direct/dresources/configs/catalogs.yml index 143a74c447d..c900ab7d66c 100644 --- a/bundle/direct/dresources/configs/catalogs.yml +++ b/bundle/direct/dresources/configs/catalogs.yml @@ -19,3 +19,6 @@ updatable_id_fields: backend_defaults: # UC auto-populates unity.catalog.managed..defaults.* keys after create. - field: properties['unity.catalog.managed.*.defaults.*'] +# Assigned by the backend and kept across renames: lets the plan find a catalog renamed outside the +# bundle or by an interrupted deploy (see InternalIDField). +internal_id_field: id diff --git a/libs/testserver/catalogs.go b/libs/testserver/catalogs.go index 6e9ed132551..4b79d8a44bf 100644 --- a/libs/testserver/catalogs.go +++ b/libs/testserver/catalogs.go @@ -6,9 +6,26 @@ import ( "net/http" "strings" + "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" ) +// fakeCatalog is CatalogInfo plus the id UC returns, which its API spec does not declare. The id +// is kept across renames. +type fakeCatalog struct { + catalog.CatalogInfo + + Id string `json:"id,omitempty"` +} + +func (c *fakeCatalog) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, c) +} + +func (c fakeCatalog) MarshalJSON() ([]byte, error) { + return marshal.Marshal(c) +} + // catalogNameManagedDefaults scopes the managed-property injection to the // backend-default drift test, keeping unrelated tests free of it. Mirrors // schemaNameManagedDefaults. @@ -83,9 +100,10 @@ func (s *FakeWorkspace) CatalogsCreate(req Request) Response { } } - s.Catalogs[createRequest.Name] = catalogInfo + created := fakeCatalog{CatalogInfo: catalogInfo, Id: nextUUID()} + s.Catalogs[createRequest.Name] = created return Response{ - Body: catalogInfo, + Body: created, } } @@ -113,7 +131,7 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { } } - applyUpdatedFields(&existing, updateRequest, fields) + applyUpdatedFields(&existing.CatalogInfo, updateRequest, fields) if updateRequest.NewName != "" { existing.Name = updateRequest.NewName existing.FullName = updateRequest.NewName diff --git a/libs/testserver/catalogs_test.go b/libs/testserver/catalogs_test.go index aa98c477479..6b121aac299 100644 --- a/libs/testserver/catalogs_test.go +++ b/libs/testserver/catalogs_test.go @@ -38,7 +38,7 @@ func TestCatalogsCreate_AllowsNonEmptyName(t *testing.T) { getResponse := MapGet(workspace, workspace.Catalogs, "my_catalog") require.Equal(t, 0, getResponse.StatusCode) - body, ok := getResponse.Body.(catalog.CatalogInfo) + body, ok := getResponse.Body.(fakeCatalog) require.True(t, ok) assert.Equal(t, "my_catalog", body.Name) assert.Equal(t, "my_catalog", body.FullName) @@ -78,8 +78,9 @@ func TestCatalogsCreate_EchoesEveryAcceptedField(t *testing.T) { response := workspace.CatalogsCreate(Request{Body: []byte(createCatalogRequest)}) assert.Equal(t, 0, response.StatusCode) - info, ok := response.Body.(catalog.CatalogInfo) + body, ok := response.Body.(fakeCatalog) require.True(t, ok) + info := body.CatalogInfo assert.Equal(t, "my_catalog", info.Name) assert.Equal(t, "c", info.Comment) @@ -123,8 +124,9 @@ func TestCatalogsUpdate_AppliesUpdatableFields(t *testing.T) { }`)}, "my_catalog") assert.Equal(t, 0, response.StatusCode) - info, ok := response.Body.(catalog.CatalogInfo) + body, ok := response.Body.(fakeCatalog) require.True(t, ok) + info := body.CatalogInfo assert.Equal(t, "updated", info.Comment) assert.Equal(t, int64(72), info.CustomMaxRetentionHours) @@ -133,3 +135,16 @@ func TestCatalogsUpdate_AppliesUpdatableFields(t *testing.T) { assert.Equal(t, map[string]string{"opt": "3"}, info.Options) assert.Equal(t, map[string]string{"prop": "4"}, info.Properties) } + +func TestCatalogsUpdate_RenameKeepsID(t *testing.T) { + workspace := NewFakeWorkspace("http://test", "dbapi123") + + created, ok := workspace.CatalogsCreate(Request{Body: []byte(`{"name": "my_catalog"}`)}).Body.(fakeCatalog) + require.True(t, ok) + require.NotEmpty(t, created.Id) + + renamed, ok := workspace.CatalogsUpdate(Request{Body: []byte(`{"new_name": "my_catalog2"}`)}, "my_catalog").Body.(fakeCatalog) + require.True(t, ok) + assert.Equal(t, "my_catalog2", renamed.Name) + assert.Equal(t, created.Id, renamed.Id) +} diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index c48ebb04df2..9cb813673de 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -241,7 +241,7 @@ type FakeWorkspace struct { ClusterLibraries map[string][]compute.Library // cluster id -> installed libraries InstancePools map[string]compute.GetInstancePool ClusterPolicies map[string]compute.Policy - Catalogs map[string]catalog.CatalogInfo + Catalogs map[string]fakeCatalog ExternalLocations map[string]catalog.ExternalLocationInfo RegisteredModels map[string]catalog.RegisteredModelInfo ModelServices map[string]catalog.ModelService @@ -535,7 +535,7 @@ func NewFakeWorkspace(url, token string) *FakeWorkspace { Monitors: map[string]catalog.MonitorInfo{}, DataQualityMonitors: map[string]dataquality.Monitor{}, Apps: map[string]apps.App{}, - Catalogs: map[string]catalog.CatalogInfo{}, + Catalogs: map[string]fakeCatalog{}, ExternalLocations: map[string]catalog.ExternalLocationInfo{}, Schemas: map[string]catalog.SchemaInfo{}, RegisteredModels: map[string]catalog.RegisteredModelInfo{},