diff --git a/acceptance/bundle/migrate/auto/registered-model-catalog-change/output.txt b/acceptance/bundle/migrate/auto/registered-model-catalog-change/output.txt index a824176b8ed..40f559184ca 100644 --- a/acceptance/bundle/migrate/auto/registered-model-catalog-change/output.txt +++ b/acceptance/bundle/migrate/auto/registered-model-catalog-change/output.txt @@ -1,9 +1,4 @@ ->>> [CLI] catalogs create othercat_[UNIQUE_NAME] -o json -{ - "name": "othercat_[UNIQUE_NAME]" -} - === Replay the prior terraform deployment (captured from a real terraform run) >>> replay_tfstate.py terraform-requests.json terraform.tfstate diff --git a/acceptance/bundle/migrate/auto/registered-model-catalog-change/script b/acceptance/bundle/migrate/auto/registered-model-catalog-change/script index d260fc05a3d..e940c7d3ca9 100644 --- a/acceptance/bundle/migrate/auto/registered-model-catalog-change/script +++ b/acceptance/bundle/migrate/auto/registered-model-catalog-change/script @@ -1,10 +1,10 @@ envsubst < databricks.yml.tmpl > databricks.yml # catalog_name is an immutable provided id field on registered models; moving the model to -# another catalog recreates it. Create the destination catalog out of band (it auto-provisions -# a "default" schema, which the moved model reuses). +# another catalog recreates it. The destination catalog is created out of band (it +# auto-provisions a "default" schema, which the moved model reuses); the captured run did that +# before the terraform deploy, so the replay below creates it. TARGET_CATALOG="othercat_$UNIQUE_NAME" -trace $CLI catalogs create "$TARGET_CATALOG" -o json | jq '{name: (.name // .full_name)}' cleanup() { title "Cleanup" diff --git a/acceptance/bundle/migrate/auto/schema-catalog-change/output.txt b/acceptance/bundle/migrate/auto/schema-catalog-change/output.txt index 5295feaa2e9..d49b7055852 100644 --- a/acceptance/bundle/migrate/auto/schema-catalog-change/output.txt +++ b/acceptance/bundle/migrate/auto/schema-catalog-change/output.txt @@ -1,9 +1,4 @@ ->>> [CLI] catalogs create othercat_[UNIQUE_NAME] -o json -{ - "name": "othercat_[UNIQUE_NAME]" -} - === Replay the prior terraform deployment (captured from a real terraform run) >>> replay_tfstate.py terraform-requests.json terraform.tfstate diff --git a/acceptance/bundle/migrate/auto/schema-catalog-change/script b/acceptance/bundle/migrate/auto/schema-catalog-change/script index 3f166db2b19..87f0e83d7f1 100644 --- a/acceptance/bundle/migrate/auto/schema-catalog-change/script +++ b/acceptance/bundle/migrate/auto/schema-catalog-change/script @@ -1,10 +1,10 @@ envsubst < databricks.yml.tmpl > databricks.yml # catalog_name is an immutable provided id field. Moving the schema to another catalog -# recreates it. Create the destination catalog out of band (a catalog cannot be a bundle -# resource here: catalogs are direct-only and this bundle deploys on terraform first). +# recreates it. The destination catalog is created out of band (a catalog cannot be a bundle +# resource here: catalogs are direct-only and this bundle deploys on terraform first); the +# captured run did that before the terraform deploy, so the replay below creates it. TARGET_CATALOG="othercat_$UNIQUE_NAME" -trace $CLI catalogs create "$TARGET_CATALOG" -o json | jq '{name: (.name // .full_name)}' cleanup() { title "Cleanup" diff --git a/acceptance/cmd/workspace/uc-renames/out.test.toml b/acceptance/cmd/workspace/uc-renames/out.test.toml new file mode 100644 index 00000000000..c7c6f58ed6e --- /dev/null +++ b/acceptance/cmd/workspace/uc-renames/out.test.toml @@ -0,0 +1 @@ +Cloud = true diff --git a/acceptance/cmd/workspace/uc-renames/output.txt b/acceptance/cmd/workspace/uc-renames/output.txt new file mode 100644 index 00000000000..7c482c7a7e3 --- /dev/null +++ b/acceptance/cmd/workspace/uc-renames/output.txt @@ -0,0 +1,104 @@ + +=== Create a catalog with a schema, and a volume, a registered model and a secret in it +>>> [CLI] catalogs create test_catalog_[UNIQUE_NAME] +{ + "name": "test_catalog_[UNIQUE_NAME]" +} + +>>> [CLI] schemas create myschema test_catalog_[UNIQUE_NAME] +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema" +} + +>>> [CLI] volumes create test_catalog_[UNIQUE_NAME] myschema myvolume MANAGED +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema.myvolume" +} + +>>> [CLI] registered-models create --catalog-name test_catalog_[UNIQUE_NAME] --schema-name myschema --name mymodel +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema.mymodel" +} + +>>> [CLI] secrets-uc create-secret mysecret test_catalog_[UNIQUE_NAME] myschema value +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema.mysecret" +} + +=== Creating a catalog under a taken name fails +>>> musterr [CLI] catalogs create test_catalog_[UNIQUE_NAME] +Error: Catalog 'test_catalog_[UNIQUE_NAME]' already exists + +=== Renaming the volume lowercases the new name +>>> [CLI] volumes update test_catalog_[UNIQUE_NAME].myschema.myvolume --new-name MyVolume2 +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema.myvolume2", + "name": "myvolume2" +} + +=== Renaming the model keeps its full name in sync +>>> [CLI] registered-models update test_catalog_[UNIQUE_NAME].myschema.mymodel --new-name mymodel2 +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema.mymodel2", + "name": "mymodel2" +} + +=== Renaming the schema moves the volume, the model and the secret with it +>>> [CLI] schemas update test_catalog_[UNIQUE_NAME].myschema --new-name myschema2 +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema2" +} + +>>> [CLI] volumes read test_catalog_[UNIQUE_NAME].myschema2.myvolume2 +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema2.myvolume2" +} + +>>> [CLI] registered-models get test_catalog_[UNIQUE_NAME].myschema2.mymodel2 +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema2.mymodel2" +} + +>>> [CLI] secrets-uc get-secret test_catalog_[UNIQUE_NAME].myschema2.mysecret +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema2.mysecret" +} + +=== Renaming the catalog moves everything in it +>>> [CLI] catalogs update test_catalog_[UNIQUE_NAME] --new-name test_catalog_renamed_[UNIQUE_NAME] +{ + "name": "test_catalog_renamed_[UNIQUE_NAME]" +} + +>>> [CLI] schemas get test_catalog_renamed_[UNIQUE_NAME].myschema2 +{ + "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema2" +} + +>>> [CLI] volumes read test_catalog_renamed_[UNIQUE_NAME].myschema2.myvolume2 +{ + "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema2.myvolume2" +} + +>>> [CLI] registered-models get test_catalog_renamed_[UNIQUE_NAME].myschema2.mymodel2 +{ + "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema2.mymodel2" +} + +>>> [CLI] secrets-uc get-secret test_catalog_renamed_[UNIQUE_NAME].myschema2.mysecret +{ + "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema2.mysecret" +} + +=== Deleting the schema with force deletes everything in it +volume: gone +model: gone +secret: gone + +=== Deleting the catalog with force deletes its schemas + +>>> [CLI] schemas create other test_catalog_renamed_[UNIQUE_NAME] +{ + "full_name": "test_catalog_renamed_[UNIQUE_NAME].other" +} +schema: gone diff --git a/acceptance/cmd/workspace/uc-renames/script b/acceptance/cmd/workspace/uc-renames/script new file mode 100644 index 00000000000..68bbc7a575c --- /dev/null +++ b/acceptance/cmd/workspace/uc-renames/script @@ -0,0 +1,54 @@ +CATALOG="test_catalog_${UNIQUE_NAME}" +RENAMED="test_catalog_renamed_${UNIQUE_NAME}" + +cleanup() { + # Whichever name the catalog ended up with. + errcode $CLI catalogs delete "$CATALOG" --force &> LOG.cleanup + errcode $CLI catalogs delete "$RENAMED" --force >> LOG.cleanup 2>&1 +} +trap cleanup EXIT + +exists() { + "$@" &> /dev/null && echo "exists" || echo "gone" +} + +title "Create a catalog with a schema, and a volume, a registered model and a secret in it" +trace $CLI catalogs create "$CATALOG" | jq '{name}' +trace $CLI schemas create myschema "$CATALOG" | jq '{full_name}' +trace $CLI volumes create "$CATALOG" myschema myvolume MANAGED | jq '{full_name}' +trace $CLI registered-models create --catalog-name "$CATALOG" --schema-name myschema --name mymodel | jq '{full_name}' +trace $CLI secrets-uc create-secret mysecret "$CATALOG" myschema value | jq '{full_name}' + +title "Creating a catalog under a taken name fails" +trace musterr $CLI catalogs create "$CATALOG" + +title "Renaming the volume lowercases the new name" +trace $CLI volumes update "$CATALOG.myschema.myvolume" --new-name MyVolume2 | jq '{full_name, name}' + +title "Renaming the model keeps its full name in sync" +trace $CLI registered-models update "$CATALOG.myschema.mymodel" --new-name mymodel2 | jq '{full_name, name}' + +title "Renaming the schema moves the volume, the model and the secret with it" +trace $CLI schemas update "$CATALOG.myschema" --new-name myschema2 | jq '{full_name}' +trace $CLI volumes read "$CATALOG.myschema2.myvolume2" | jq '{full_name}' +trace $CLI registered-models get "$CATALOG.myschema2.mymodel2" | jq '{full_name}' +trace $CLI secrets-uc get-secret "$CATALOG.myschema2.mysecret" | jq '{full_name}' + +title "Renaming the catalog moves everything in it" +trace $CLI catalogs update "$CATALOG" --new-name "$RENAMED" | jq '{name}' +trace $CLI schemas get "$RENAMED.myschema2" | jq '{full_name}' +trace $CLI volumes read "$RENAMED.myschema2.myvolume2" | jq '{full_name}' +trace $CLI registered-models get "$RENAMED.myschema2.mymodel2" | jq '{full_name}' +trace $CLI secrets-uc get-secret "$RENAMED.myschema2.mysecret" | jq '{full_name}' + +title "Deleting the schema with force deletes everything in it\n" +# Not traced: the "not found" messages differ between UC and the test server. +$CLI schemas delete "$RENAMED.myschema2" --force +echo "volume: $(exists $CLI volumes read "$RENAMED.myschema2.myvolume2")" +echo "model: $(exists $CLI registered-models get "$RENAMED.myschema2.mymodel2")" +echo "secret: $(exists $CLI secrets-uc get-secret "$RENAMED.myschema2.mysecret")" + +title "Deleting the catalog with force deletes its schemas\n" +trace $CLI schemas create other "$RENAMED" | jq '{full_name}' +$CLI catalogs delete "$RENAMED" --force +echo "schema: $(exists $CLI schemas get "$RENAMED.other")" diff --git a/acceptance/cmd/workspace/uc-renames/test.toml b/acceptance/cmd/workspace/uc-renames/test.toml new file mode 100644 index 00000000000..b86a07771e2 --- /dev/null +++ b/acceptance/cmd/workspace/uc-renames/test.toml @@ -0,0 +1,2 @@ +# Checks the test server's Unity Catalog renames and deletes against a real workspace. +Cloud = true diff --git a/libs/testserver/catalogs.go b/libs/testserver/catalogs.go index 9759b4b1a19..6e9ed132551 100644 --- a/libs/testserver/catalogs.go +++ b/libs/testserver/catalogs.go @@ -37,6 +37,16 @@ func (s *FakeWorkspace) CatalogsCreate(req Request) Response { } } + if _, exists := s.Catalogs[createRequest.Name]; exists { + return Response{ + StatusCode: http.StatusBadRequest, + Body: map[string]string{ + "error_code": "CATALOG_ALREADY_EXISTS", + "message": fmt.Sprintf("Catalog '%s' already exists", createRequest.Name), + }, + } + } + // Echo back every field create accepts: a dropped one makes the next plan see a // phantom change. catalogInfo := catalog.CatalogInfo{ @@ -110,6 +120,7 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { // Delete the old entry and create with new name delete(s.Catalogs, name) + s.moveCatalogChildren(name, updateRequest.NewName) name = updateRequest.NewName } @@ -121,3 +132,75 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { Body: existing, } } + +// moveCatalogChildren mirrors UC, where the schemas, volumes, registered models and secrets of a renamed catalog move with it. +func (s *FakeWorkspace) moveCatalogChildren(oldCatalogName, newCatalogName string) { + for key, schema := range s.Schemas { + if schema.CatalogName != oldCatalogName { + continue + } + delete(s.Schemas, key) + schema.CatalogName = newCatalogName + schema.FullName = newCatalogName + "." + schema.Name + s.Schemas[schema.FullName] = schema + } + for key, volume := range s.Volumes { + if volume.CatalogName != oldCatalogName { + continue + } + delete(s.Volumes, key) + volume.CatalogName = newCatalogName + volume.FullName = newCatalogName + "." + volume.SchemaName + "." + volume.Name + s.Volumes[volume.FullName] = volume + } + for key, model := range s.RegisteredModels { + if model.CatalogName != oldCatalogName { + continue + } + model.CatalogName = newCatalogName + s.moveRegisteredModel(key, model) + } + for key, secret := range s.UCSecrets { + if secret.CatalogName != oldCatalogName { + continue + } + delete(s.UCSecrets, key) + secret.CatalogName = newCatalogName + secret.FullName = newCatalogName + "." + secret.SchemaName + "." + secret.Name + s.UCSecrets[secret.FullName] = secret + } +} + +// CatalogsDelete mirrors UC, where deleting a catalog with force deletes its schemas, volumes, registered models and secrets too. +func (s *FakeWorkspace) CatalogsDelete(req Request, name string) Response { + defer s.LockUnlock()() + + if _, ok := s.Catalogs[name]; !ok { + return Response{StatusCode: http.StatusNotFound} + } + delete(s.Catalogs, name) + if req.URL.Query().Get("force") == "true" { + for key, schema := range s.Schemas { + if schema.CatalogName == name { + delete(s.Schemas, key) + } + } + for key, volume := range s.Volumes { + if volume.CatalogName == name { + delete(s.Volumes, key) + } + } + for key, model := range s.RegisteredModels { + if model.CatalogName == name { + delete(s.RegisteredModels, key) + delete(s.RegisteredModelVersions, key) + } + } + for key, secret := range s.UCSecrets { + if secret.CatalogName == name { + delete(s.UCSecrets, key) + } + } + } + return Response{} +} diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index e5f4c37a16b..1fa448f4743 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -798,7 +798,7 @@ func AddDefaultHandlers(server *Server) { }) server.Handle("DELETE", "/api/2.1/unity-catalog/schemas/{full_name}", func(req Request) any { - return MapDelete(req.Workspace, req.Workspace.Schemas, req.Vars["full_name"]) + return req.Workspace.SchemasDelete(req, req.Vars["full_name"]) }) // Grants: @@ -826,7 +826,7 @@ func AddDefaultHandlers(server *Server) { }) server.Handle("DELETE", "/api/2.1/unity-catalog/catalogs/{name}", func(req Request) any { - return MapDelete(req.Workspace, req.Workspace.Catalogs, req.Vars["name"]) + return req.Workspace.CatalogsDelete(req, req.Vars["name"]) }) // External Locations: diff --git a/libs/testserver/registered_models.go b/libs/testserver/registered_models.go index 7e178e4b29b..2dea8ecc864 100644 --- a/libs/testserver/registered_models.go +++ b/libs/testserver/registered_models.go @@ -77,25 +77,46 @@ func (s *FakeWorkspace) RegisteredModelsUpdate(req Request, fullName string) Res } } + if updateRequest.NewName != "" && updateRequest.Name != "" { + return Response{ + StatusCode: http.StatusBadRequest, + Body: map[string]string{ + "error_code": "INVALID_PARAMETER_VALUE", + "message": "UpdateRegisteredModel Both new_name and name cannot be specified.", + }, + } + } + // The update API ignores aliases; they are set through the alias endpoints. delete(fields, "aliases") applyUpdatedFields(&existing, updateRequest, fields) if updateRequest.NewName != "" { existing.Name = updateRequest.NewName - - // Delete the old entry and set full name to the new name - delete(s.RegisteredModels, fullName) - fullName = existing.CatalogName + "." + existing.SchemaName + "." + updateRequest.NewName } existing.UpdatedAt = nowMilli() - s.RegisteredModels[fullName] = existing + existing = s.moveRegisteredModel(fullName, existing) return Response{ Body: existing, } } +// moveRegisteredModel stores the model under the full name its catalog, schema and name now give it, +// keeping its versions, as UC does when the model or its parent is renamed. +func (s *FakeWorkspace) moveRegisteredModel(oldFullName string, model catalog.RegisteredModelInfo) catalog.RegisteredModelInfo { + delete(s.RegisteredModels, oldFullName) + model.FullName = model.CatalogName + "." + model.SchemaName + "." + model.Name + s.RegisteredModels[model.FullName] = model + + versions, ok := s.RegisteredModelVersions[oldFullName] + if ok { + delete(s.RegisteredModelVersions, oldFullName) + s.RegisteredModelVersions[model.FullName] = versions + } + return model +} + // RegisteredModelsGet mirrors UC, which lists a model's aliases only when include_aliases is set. func (s *FakeWorkspace) RegisteredModelsGet(req Request, fullName string) Response { defer s.LockUnlock()() diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index 8094c93acc6..78fdd13903f 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -86,7 +86,7 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { return *errResponse } - var schemaUpdate catalog.SchemaInfo + var schemaUpdate catalog.UpdateSchema if err := json.Unmarshal(req.Body, &schemaUpdate); err != nil { return Response{ @@ -97,12 +97,80 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { applyUpdatedFields(&existing, schemaUpdate, fields) + if schemaUpdate.NewName != "" { + delete(s.Schemas, name) + oldName := existing.Name + // UC normalizes schema names to lowercase. + existing.Name = strings.ToLower(schemaUpdate.NewName) + existing.FullName = existing.CatalogName + "." + existing.Name + s.moveSchemaChildren(existing.CatalogName, oldName, existing.Name) + } + existing.UpdatedAt = nowMilli() existing.UpdatedBy = s.CurrentUser().UserName - s.Schemas[name] = existing + s.Schemas[existing.FullName] = existing return Response{ Body: existing, } } + +// moveSchemaChildren mirrors UC, where the volumes, registered models and secrets of a renamed schema move with it. +func (s *FakeWorkspace) moveSchemaChildren(catalogName, oldSchemaName, newSchemaName string) { + for key, volume := range s.Volumes { + if volume.CatalogName != catalogName || volume.SchemaName != oldSchemaName { + continue + } + delete(s.Volumes, key) + volume.SchemaName = newSchemaName + volume.FullName = catalogName + "." + newSchemaName + "." + volume.Name + s.Volumes[volume.FullName] = volume + } + for key, model := range s.RegisteredModels { + if model.CatalogName != catalogName || model.SchemaName != oldSchemaName { + continue + } + model.SchemaName = newSchemaName + s.moveRegisteredModel(key, model) + } + for key, secret := range s.UCSecrets { + if secret.CatalogName != catalogName || secret.SchemaName != oldSchemaName { + continue + } + delete(s.UCSecrets, key) + secret.SchemaName = newSchemaName + secret.FullName = catalogName + "." + newSchemaName + "." + secret.Name + s.UCSecrets[secret.FullName] = secret + } +} + +// SchemasDelete mirrors UC, where deleting a schema with force deletes the volumes, registered models and secrets in it too. +func (s *FakeWorkspace) SchemasDelete(req Request, fullName string) Response { + defer s.LockUnlock()() + + schema, ok := s.Schemas[fullName] + if !ok { + return Response{StatusCode: http.StatusNotFound} + } + delete(s.Schemas, fullName) + if req.URL.Query().Get("force") == "true" { + for key, volume := range s.Volumes { + if volume.CatalogName == schema.CatalogName && volume.SchemaName == schema.Name { + delete(s.Volumes, key) + } + } + for key, model := range s.RegisteredModels { + if model.CatalogName == schema.CatalogName && model.SchemaName == schema.Name { + delete(s.RegisteredModels, key) + delete(s.RegisteredModelVersions, key) + } + } + for key, secret := range s.UCSecrets { + if secret.CatalogName == schema.CatalogName && secret.SchemaName == schema.Name { + delete(s.UCSecrets, key) + } + } + } + return Response{} +} diff --git a/libs/testserver/volumes.go b/libs/testserver/volumes.go index 91f29a94e4e..00466f88b6f 100644 --- a/libs/testserver/volumes.go +++ b/libs/testserver/volumes.go @@ -94,8 +94,9 @@ func (s *FakeWorkspace) VolumesUpdate(req Request, fullname string) Response { if request.NewName != "" { delete(s.Volumes, fullname) - existing.Name = request.NewName - existing.FullName = existing.CatalogName + "." + existing.SchemaName + "." + request.NewName + // UC lowercases the new name, as on create. + existing.Name = strings.ToLower(request.NewName) + existing.FullName = existing.CatalogName + "." + existing.SchemaName + "." + existing.Name } existing.UpdatedAt = nowMilli()