diff --git a/acceptance/bundle/resources/catalogs/empty-name/databricks.yml.tmpl b/acceptance/bundle/resources/catalogs/empty-name/databricks.yml.tmpl new file mode 100644 index 00000000000..290f0ef2d0d --- /dev/null +++ b/acceptance/bundle/resources/catalogs/empty-name/databricks.yml.tmpl @@ -0,0 +1,7 @@ +bundle: + name: catalog-empty-name-$UNIQUE_NAME + +resources: + catalogs: + mycatalog: + name: "" diff --git a/acceptance/bundle/resources/catalogs/empty-name/out.test.toml b/acceptance/bundle/resources/catalogs/empty-name/out.test.toml new file mode 100644 index 00000000000..fe4076cdf9b --- /dev/null +++ b/acceptance/bundle/resources/catalogs/empty-name/out.test.toml @@ -0,0 +1,4 @@ +Local = true +Cloud = true +RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/catalogs/empty-name/output.txt b/acceptance/bundle/resources/catalogs/empty-name/output.txt new file mode 100644 index 00000000000..0a21066c99b --- /dev/null +++ b/acceptance/bundle/resources/catalogs/empty-name/output.txt @@ -0,0 +1,11 @@ + +>>> musterr [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/catalog-empty-name-[UNIQUE_NAME]/default/files... +Deploying resources... +Error: cannot create resources.catalogs.mycatalog: Invalid input: RPC CreateCatalog Field managedcatalog.CatalogInfo.name: name "" is not a valid name. Valid names cannot contain spaces, periods, forward slashes, or control characters. (400 INVALID_PARAMETER_VALUE) + +Endpoint: POST [DATABRICKS_URL]/api/2.1/unity-catalog/catalogs +HTTP Status: 400 Bad Request +API error_code: INVALID_PARAMETER_VALUE +API message: Invalid input: RPC CreateCatalog Field managedcatalog.CatalogInfo.name: name "" is not a valid name. Valid names cannot contain spaces, periods, forward slashes, or control characters. + diff --git a/acceptance/bundle/resources/catalogs/empty-name/script b/acceptance/bundle/resources/catalogs/empty-name/script new file mode 100644 index 00000000000..dc9e56639a9 --- /dev/null +++ b/acceptance/bundle/resources/catalogs/empty-name/script @@ -0,0 +1,5 @@ +# The deploy fails only after the files are uploaded, so $UNIQUE_NAME in the bundle +# name keeps concurrent cloud legs apart and lets the sweeper find what is left. +envsubst < databricks.yml.tmpl > databricks.yml + +trace musterr $CLI bundle deploy diff --git a/acceptance/bundle/resources/catalogs/empty-name/test.toml b/acceptance/bundle/resources/catalogs/empty-name/test.toml new file mode 100644 index 00000000000..857f971d215 --- /dev/null +++ b/acceptance/bundle/resources/catalogs/empty-name/test.toml @@ -0,0 +1,10 @@ +Local = true +# The golden asserts UC's message verbatim, so run on cloud to catch it drifting. +Cloud = true +RequiresUnityCatalog = true +RecordRequests = false +Ignore = [".databricks"] + +# Terraform rejects catalog resources before any API call, so there is nothing to +# assert on that engine. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/models/empty-name/databricks.yml.tmpl b/acceptance/bundle/resources/models/empty-name/databricks.yml.tmpl new file mode 100644 index 00000000000..a5fd377721f --- /dev/null +++ b/acceptance/bundle/resources/models/empty-name/databricks.yml.tmpl @@ -0,0 +1,7 @@ +bundle: + name: model-empty-name-$UNIQUE_NAME + +resources: + models: + mymodel: + name: "" diff --git a/acceptance/bundle/resources/models/empty-name/out.deploy.direct.txt b/acceptance/bundle/resources/models/empty-name/out.deploy.direct.txt new file mode 100644 index 00000000000..ee04aaab7dc --- /dev/null +++ b/acceptance/bundle/resources/models/empty-name/out.deploy.direct.txt @@ -0,0 +1,11 @@ + +>>> musterr [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/model-empty-name-[UNIQUE_NAME]/default/files... +Deploying resources... +Error: cannot create resources.models.mymodel: Got an invalid name ''. Registered Model names cannot be empty strings. (400 INVALID_PARAMETER_VALUE) + +Endpoint: POST [DATABRICKS_URL]/api/2.0/mlflow/registered-models/create +HTTP Status: 400 Bad Request +API error_code: INVALID_PARAMETER_VALUE +API message: Got an invalid name ''. Registered Model names cannot be empty strings. + diff --git a/acceptance/bundle/resources/models/empty-name/out.deploy.terraform.txt b/acceptance/bundle/resources/models/empty-name/out.deploy.terraform.txt new file mode 100644 index 00000000000..80c84c93bf3 --- /dev/null +++ b/acceptance/bundle/resources/models/empty-name/out.deploy.terraform.txt @@ -0,0 +1,15 @@ + +>>> musterr [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/model-empty-name-[UNIQUE_NAME]/default/files... +Deploying resources... +Error: terraform apply: exit status 1 + +Error: cannot create mlflow model: Got an invalid name ''. Registered Model names cannot be empty strings. + + with databricks_mlflow_model.mymodel, + on bundle.tf.json line 17, in resource.databricks_mlflow_model.mymodel: + 17: } + + + +Updating deployment state... diff --git a/acceptance/bundle/resources/models/empty-name/out.test.toml b/acceptance/bundle/resources/models/empty-name/out.test.toml new file mode 100644 index 00000000000..bbc7fcfd1bd --- /dev/null +++ b/acceptance/bundle/resources/models/empty-name/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/resources/models/empty-name/output.txt b/acceptance/bundle/resources/models/empty-name/output.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/acceptance/bundle/resources/models/empty-name/script b/acceptance/bundle/resources/models/empty-name/script new file mode 100644 index 00000000000..0336c9c6ca0 --- /dev/null +++ b/acceptance/bundle/resources/models/empty-name/script @@ -0,0 +1,7 @@ +# The deploy fails only after the files are uploaded, so $UNIQUE_NAME in the bundle +# name keeps concurrent cloud legs apart and lets the sweeper find what is left. +envsubst < databricks.yml.tmpl > databricks.yml + +# Both engines reach the create call, but terraform wraps the message in its own +# output, so the goldens are per-engine. +trace musterr $CLI bundle deploy &> out.deploy.$DATABRICKS_BUNDLE_ENGINE.txt diff --git a/acceptance/bundle/resources/models/empty-name/test.toml b/acceptance/bundle/resources/models/empty-name/test.toml new file mode 100644 index 00000000000..94abe3cfaed --- /dev/null +++ b/acceptance/bundle/resources/models/empty-name/test.toml @@ -0,0 +1,5 @@ +Local = true +# The golden asserts MLflow's message verbatim, so run on cloud to catch it drifting. +Cloud = true +RecordRequests = false +Ignore = [".databricks"] diff --git a/libs/testserver/catalogs.go b/libs/testserver/catalogs.go index 4d80152373d..1d0bc065681 100644 --- a/libs/testserver/catalogs.go +++ b/libs/testserver/catalogs.go @@ -24,6 +24,18 @@ func (s *FakeWorkspace) CatalogsCreate(req Request) Response { } } + // UC rejects an empty name; the fake would otherwise store a catalog under a key + // nothing can look up. Message is UC's canned error, which names more than we check. + if createRequest.Name == "" { + return Response{ + StatusCode: http.StatusBadRequest, + Body: map[string]string{ + "error_code": "INVALID_PARAMETER_VALUE", + "message": `Invalid input: RPC CreateCatalog Field managedcatalog.CatalogInfo.name: name "" is not a valid name. Valid names cannot contain spaces, periods, forward slashes, or control characters.`, + }, + } + } + // Echo back every field create accepts: a dropped one makes the next plan see a // phantom change. catalogInfo := catalog.CatalogInfo{ diff --git a/libs/testserver/catalogs_test.go b/libs/testserver/catalogs_test.go index 4606fdf28b5..eb05ab8ec56 100644 --- a/libs/testserver/catalogs_test.go +++ b/libs/testserver/catalogs_test.go @@ -11,6 +11,39 @@ import ( "github.com/stretchr/testify/require" ) +func TestCatalogsCreate_RejectsEmptyName(t *testing.T) { + workspace := NewFakeWorkspace("http://test", "dbapi123") + + response := workspace.CatalogsCreate(Request{Body: []byte(`{"name": ""}`)}) + assert.Equal(t, 400, response.StatusCode) + + // A stored-but-unreadable catalog is the original bug. Asserted before the + // require below so it is still reported when the rejection is missing. + assert.Empty(t, workspace.Catalogs) + + body, ok := response.Body.(map[string]string) + require.True(t, ok) + assert.Equal(t, "INVALID_PARAMETER_VALUE", body["error_code"]) + assert.Contains(t, body["message"], "is not a valid name") +} + +func TestCatalogsCreate_AllowsNonEmptyName(t *testing.T) { + workspace := NewFakeWorkspace("http://test", "dbapi123") + + response := workspace.CatalogsCreate(Request{Body: []byte(`{"name": "my_catalog"}`)}) + // StatusCode 0 gets converted to 200 by normalizeResponse in the server + require.Equal(t, 0, response.StatusCode) + + // Read back through the same helper the GET route uses: a mis-keyed store must fail here. + getResponse := MapGet(workspace, workspace.Catalogs, "my_catalog") + require.Equal(t, 0, getResponse.StatusCode) + + body, ok := getResponse.Body.(catalog.CatalogInfo) + require.True(t, ok) + assert.Equal(t, "my_catalog", body.Name) + assert.Equal(t, "my_catalog", body.FullName) +} + // createCatalogRequest sets every field CreateCatalog accepts. Tests below assert // the fake echoes all of them back and that the request stays exhaustive. const createCatalogRequest = `{ diff --git a/libs/testserver/models.go b/libs/testserver/models.go index febfd8bf8a8..3314327d8a7 100644 --- a/libs/testserver/models.go +++ b/libs/testserver/models.go @@ -19,6 +19,18 @@ func (s *FakeWorkspace) ModelRegistryCreateModel(req Request) any { } } + // MLflow rejects an empty name; the fake would otherwise store a model under a key + // nothing can look up. + if request.Name == "" { + return Response{ + StatusCode: 400, + Body: map[string]string{ + "error_code": "INVALID_PARAMETER_VALUE", + "message": "Got an invalid name ''. Registered Model names cannot be empty strings.", + }, + } + } + // Create the model with a numeric ID (matching real API behavior) modelId := strconv.FormatInt(nextID(), 10) model := ml.Model{ diff --git a/libs/testserver/models_test.go b/libs/testserver/models_test.go new file mode 100644 index 00000000000..e72d4930133 --- /dev/null +++ b/libs/testserver/models_test.go @@ -0,0 +1,48 @@ +package testserver + +import ( + "net/url" + "testing" + + "github.com/databricks/databricks-sdk-go/service/ml" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestModelRegistryCreateModel_RejectsEmptyName(t *testing.T) { + workspace := NewFakeWorkspace("http://test", "dbapi123") + + response, ok := workspace.ModelRegistryCreateModel(Request{Body: []byte(`{"name": ""}`)}).(Response) + require.True(t, ok) + assert.Equal(t, 400, response.StatusCode) + + // A stored-but-unreadable model is the original bug. Asserted before the + // require below so it is still reported when the rejection is missing. + assert.Empty(t, workspace.ModelRegistryModels) + + body, ok := response.Body.(map[string]string) + require.True(t, ok) + assert.Equal(t, "INVALID_PARAMETER_VALUE", body["error_code"]) + assert.Contains(t, body["message"], "cannot be empty strings") +} + +func TestModelRegistryCreateModel_AllowsNonEmptyName(t *testing.T) { + workspace := NewFakeWorkspace("http://test", "dbapi123") + + response, ok := workspace.ModelRegistryCreateModel(Request{Body: []byte(`{"name": "my_model"}`)}).(Response) + require.True(t, ok) + // StatusCode 0 gets converted to 200 by normalizeResponse in the server + require.Equal(t, 0, response.StatusCode) + + // Read back through the GET handler: a mis-keyed store must fail here. + getResponse, ok := workspace.ModelRegistryGetModel(Request{ + URL: &url.URL{RawQuery: "name=my_model"}, + }).(Response) + require.True(t, ok) + require.Equal(t, 0, getResponse.StatusCode) + + body, ok := getResponse.Body.(ml.GetModelResponse) + require.True(t, ok) + assert.Equal(t, "my_model", body.RegisteredModelDatabricks.Name) + assert.NotEmpty(t, body.RegisteredModelDatabricks.Id) +}