diff --git a/.nextchanges/bundles/invalid-identifiers.md b/.nextchanges/bundles/invalid-identifiers.md new file mode 100644 index 00000000000..796251a3d74 --- /dev/null +++ b/.nextchanges/bundles/invalid-identifiers.md @@ -0,0 +1,3 @@ +Reject empty, blank, or control-character resource identifiers and incomplete pipeline library paths during bundle validation. + +Configs that previously only warned on missing names (for example models and apps) now fail validate. Explicit empty strings for UC parent fields such as catalog_name are rejected; omitted parents still warn. diff --git a/acceptance/bundle/resources/catalogs/empty-name/databricks.yml.tmpl b/acceptance/bundle/resources/catalogs/empty-name/databricks.yml.tmpl deleted file mode 100644 index 290f0ef2d0d..00000000000 --- a/acceptance/bundle/resources/catalogs/empty-name/databricks.yml.tmpl +++ /dev/null @@ -1,7 +0,0 @@ -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 deleted file mode 100644 index c502b28221b..00000000000 --- a/acceptance/bundle/resources/catalogs/empty-name/out.test.toml +++ /dev/null @@ -1,2 +0,0 @@ -Cloud = 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 deleted file mode 100644 index 51b04630435..00000000000 --- a/acceptance/bundle/resources/catalogs/empty-name/output.txt +++ /dev/null @@ -1,11 +0,0 @@ - ->>> musterr [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/catalog-empty-name-[UNIQUE_NAME]/default/files... -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. - -Files: 5 uploaded, 0 deleted diff --git a/acceptance/bundle/resources/catalogs/empty-name/script b/acceptance/bundle/resources/catalogs/empty-name/script deleted file mode 100644 index dc9e56639a9..00000000000 --- a/acceptance/bundle/resources/catalogs/empty-name/script +++ /dev/null @@ -1,5 +0,0 @@ -# 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 deleted file mode 100644 index 458319b24ad..00000000000 --- a/acceptance/bundle/resources/catalogs/empty-name/test.toml +++ /dev/null @@ -1,8 +0,0 @@ -# The golden asserts UC's message verbatim, so run on cloud to catch it drifting. -Cloud = 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 deleted file mode 100644 index a5fd377721f..00000000000 --- a/acceptance/bundle/resources/models/empty-name/databricks.yml.tmpl +++ /dev/null @@ -1,7 +0,0 @@ -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 deleted file mode 100644 index 50232dbed92..00000000000 --- a/acceptance/bundle/resources/models/empty-name/out.deploy.direct.txt +++ /dev/null @@ -1,11 +0,0 @@ - ->>> musterr [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/model-empty-name-[UNIQUE_NAME]/default/files... -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. - -Files: 6 uploaded, 0 deleted diff --git a/acceptance/bundle/resources/models/empty-name/out.deploy.terraform.txt b/acceptance/bundle/resources/models/empty-name/out.deploy.terraform.txt deleted file mode 100644 index adb61687bf8..00000000000 --- a/acceptance/bundle/resources/models/empty-name/out.deploy.terraform.txt +++ /dev/null @@ -1,14 +0,0 @@ - ->>> musterr [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/model-empty-name-[UNIQUE_NAME]/default/files... -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: } - - - -Files: 6 uploaded, 0 deleted diff --git a/acceptance/bundle/resources/models/empty-name/output.txt b/acceptance/bundle/resources/models/empty-name/output.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/acceptance/bundle/resources/models/empty-name/script b/acceptance/bundle/resources/models/empty-name/script deleted file mode 100644 index 0336c9c6ca0..00000000000 --- a/acceptance/bundle/resources/models/empty-name/script +++ /dev/null @@ -1,7 +0,0 @@ -# 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 deleted file mode 100644 index 64c48c920e6..00000000000 --- a/acceptance/bundle/resources/models/empty-name/test.toml +++ /dev/null @@ -1,4 +0,0 @@ -# The golden asserts MLflow's message verbatim, so run on cloud to catch it drifting. -Cloud = true -RecordRequests = false -Ignore = [".databricks"] diff --git a/acceptance/bundle/validate/dashboard_required_name/output.txt b/acceptance/bundle/validate/dashboard_required_name/output.txt index 3c185500b6e..353c6ce93af 100644 --- a/acceptance/bundle/validate/dashboard_required_name/output.txt +++ b/acceptance/bundle/validate/dashboard_required_name/output.txt @@ -1,7 +1,7 @@ >>> [CLI] bundle validate Error: dashboard display_name is required - at resources.dashboards.my_dashboard + at resources.dashboards.my_dashboard.display_name in databricks.yml:8:7 Name: test-bundle diff --git a/acceptance/bundle/validate/empty_resources/empty_dict/output.txt b/acceptance/bundle/validate/empty_resources/empty_dict/output.txt index 0ff8111602e..d8d55774ef7 100644 --- a/acceptance/bundle/validate/empty_resources/empty_dict/output.txt +++ b/acceptance/bundle/validate/empty_resources/empty_dict/output.txt @@ -33,8 +33,8 @@ } === resources.models.rname === -Warning: required field "name" is not set - at resources.models.rname +Error: model name is required + at resources.models.rname.name in databricks.yml:6:12 { @@ -53,6 +53,10 @@ Warning: required field "name" is not set } === resources.registered_models.rname === +Error: registered_model name is required + at resources.registered_models.rname.name + in databricks.yml:6:12 + { "registered_models": { "rname": {} @@ -79,12 +83,8 @@ Warning: required field "table_name" is not set } === resources.schemas.rname === -Warning: required field "catalog_name" is not set - at resources.schemas.rname - in databricks.yml:6:12 - -Warning: required field "name" is not set - at resources.schemas.rname +Error: schema name is required + at resources.schemas.rname.name in databricks.yml:6:12 { @@ -94,16 +94,8 @@ Warning: required field "name" is not set } === resources.volumes.rname === -Warning: required field "catalog_name" is not set - at resources.volumes.rname - in databricks.yml:6:12 - -Warning: required field "name" is not set - at resources.volumes.rname - in databricks.yml:6:12 - -Warning: required field "schema_name" is not set - at resources.volumes.rname +Error: volume name is required + at resources.volumes.rname.name in databricks.yml:6:12 { @@ -126,7 +118,7 @@ Warning: required field "schema_name" is not set === resources.dashboards.rname === Error: dashboard display_name is required - at resources.dashboards.rname + at resources.dashboards.rname.display_name in databricks.yml:6:12 Error: dashboard warehouse_id is required @@ -143,15 +135,10 @@ Error: dashboard warehouse_id is required } === resources.apps.rname === -Warning: required field "name" is not set - at resources.apps.rname - in databricks.yml:6:12 - -Error: Missing app source code path or git source +Error: app name is required + at resources.apps.rname.name in databricks.yml:6:12 -app resource 'rname' should have either source_code_path or git_source field - { "apps": { "rname": { @@ -162,7 +149,7 @@ app resource 'rname' should have either source_code_path or git_source field === resources.sql_warehouses.rname === Error: sql_warehouse name is required - at resources.sql_warehouses.rname + at resources.sql_warehouses.rname.name in databricks.yml:6:12 { @@ -177,8 +164,8 @@ Error: sql_warehouse name is required } === resources.secret_scopes.rname === -Warning: required field "name" is not set - at resources.secret_scopes.rname +Error: secret_scope name is required + at resources.secret_scopes.rname.name in databricks.yml:6:12 { @@ -188,24 +175,8 @@ Warning: required field "name" is not set } === resources.alerts.rname === -Warning: required field "display_name" is not set - at resources.alerts.rname - in databricks.yml:6:12 - -Warning: required field "evaluation" is not set - at resources.alerts.rname - in databricks.yml:6:12 - -Warning: required field "query_text" is not set - at resources.alerts.rname - in databricks.yml:6:12 - -Warning: required field "schedule" is not set - at resources.alerts.rname - in databricks.yml:6:12 - -Warning: required field "warehouse_id" is not set - at resources.alerts.rname +Error: alert display_name is required + at resources.alerts.rname.display_name in databricks.yml:6:12 { diff --git a/acceptance/bundle/validate/empty_resources/with_grants/output.txt b/acceptance/bundle/validate/empty_resources/with_grants/output.txt index 38c4dc55d19..2c75a041318 100644 --- a/acceptance/bundle/validate/empty_resources/with_grants/output.txt +++ b/acceptance/bundle/validate/empty_resources/with_grants/output.txt @@ -45,8 +45,8 @@ Warning: unknown field: grants at resources.models.rname in databricks.yml:7:7 -Warning: required field "name" is not set - at resources.models.rname +Error: model name is required + at resources.models.rname.name in databricks.yml:7:7 { @@ -69,6 +69,10 @@ Warning: unknown field: grants } === resources.registered_models.rname === +Error: registered_model name is required + at resources.registered_models.rname.name + in databricks.yml:7:7 + { "registered_models": { "rname": { @@ -101,12 +105,8 @@ Warning: required field "table_name" is not set } === resources.schemas.rname === -Warning: required field "catalog_name" is not set - at resources.schemas.rname - in databricks.yml:7:7 - -Warning: required field "name" is not set - at resources.schemas.rname +Error: schema name is required + at resources.schemas.rname.name in databricks.yml:7:7 { @@ -118,16 +118,8 @@ Warning: required field "name" is not set } === resources.volumes.rname === -Warning: required field "catalog_name" is not set - at resources.volumes.rname - in databricks.yml:7:7 - -Warning: required field "name" is not set - at resources.volumes.rname - in databricks.yml:7:7 - -Warning: required field "schema_name" is not set - at resources.volumes.rname +Error: volume name is required + at resources.volumes.rname.name in databricks.yml:7:7 { @@ -159,7 +151,7 @@ Warning: unknown field: grants in databricks.yml:7:7 Error: dashboard display_name is required - at resources.dashboards.rname + at resources.dashboards.rname.display_name in databricks.yml:7:7 Error: dashboard warehouse_id is required @@ -180,15 +172,10 @@ Warning: unknown field: grants at resources.apps.rname in databricks.yml:7:7 -Warning: required field "name" is not set - at resources.apps.rname +Error: app name is required + at resources.apps.rname.name in databricks.yml:7:7 -Error: Missing app source code path or git source - in databricks.yml:7:7 - -app resource 'rname' should have either source_code_path or git_source field - { "apps": { "rname": { @@ -203,7 +190,7 @@ Warning: unknown field: grants in databricks.yml:7:7 Error: sql_warehouse name is required - at resources.sql_warehouses.rname + at resources.sql_warehouses.rname.name in databricks.yml:7:7 { @@ -222,8 +209,8 @@ Warning: unknown field: grants at resources.secret_scopes.rname in databricks.yml:7:7 -Warning: required field "name" is not set - at resources.secret_scopes.rname +Error: secret_scope name is required + at resources.secret_scopes.rname.name in databricks.yml:7:7 { @@ -237,24 +224,8 @@ Warning: unknown field: grants at resources.alerts.rname in databricks.yml:7:7 -Warning: required field "display_name" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "evaluation" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "query_text" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "schedule" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "warehouse_id" is not set - at resources.alerts.rname +Error: alert display_name is required + at resources.alerts.rname.display_name in databricks.yml:7:7 { diff --git a/acceptance/bundle/validate/empty_resources/with_permissions/output.txt b/acceptance/bundle/validate/empty_resources/with_permissions/output.txt index cef0b18baa3..bd0bfe3549c 100644 --- a/acceptance/bundle/validate/empty_resources/with_permissions/output.txt +++ b/acceptance/bundle/validate/empty_resources/with_permissions/output.txt @@ -33,8 +33,8 @@ } === resources.models.rname === -Warning: required field "name" is not set - at resources.models.rname +Error: model name is required + at resources.models.rname.name in databricks.yml:7:7 { @@ -57,6 +57,10 @@ Warning: unknown field: permissions at resources.registered_models.rname in databricks.yml:7:7 +Error: registered_model name is required + at resources.registered_models.rname.name + in databricks.yml:7:7 + { "registered_models": { "rname": {} @@ -91,12 +95,8 @@ Warning: unknown field: permissions at resources.schemas.rname in databricks.yml:7:7 -Warning: required field "catalog_name" is not set - at resources.schemas.rname - in databricks.yml:7:7 - -Warning: required field "name" is not set - at resources.schemas.rname +Error: schema name is required + at resources.schemas.rname.name in databricks.yml:7:7 { @@ -110,16 +110,8 @@ Warning: unknown field: permissions at resources.volumes.rname in databricks.yml:7:7 -Warning: required field "catalog_name" is not set - at resources.volumes.rname - in databricks.yml:7:7 - -Warning: required field "name" is not set - at resources.volumes.rname - in databricks.yml:7:7 - -Warning: required field "schema_name" is not set - at resources.volumes.rname +Error: volume name is required + at resources.volumes.rname.name in databricks.yml:7:7 { @@ -142,7 +134,7 @@ Warning: required field "schema_name" is not set === resources.dashboards.rname === Error: dashboard display_name is required - at resources.dashboards.rname + at resources.dashboards.rname.display_name in databricks.yml:7:7 Error: dashboard warehouse_id is required @@ -159,15 +151,10 @@ Error: dashboard warehouse_id is required } === resources.apps.rname === -Warning: required field "name" is not set - at resources.apps.rname - in databricks.yml:7:7 - -Error: Missing app source code path or git source +Error: app name is required + at resources.apps.rname.name in databricks.yml:7:7 -app resource 'rname' should have either source_code_path or git_source field - { "apps": { "rname": { @@ -178,7 +165,7 @@ app resource 'rname' should have either source_code_path or git_source field === resources.sql_warehouses.rname === Error: sql_warehouse name is required - at resources.sql_warehouses.rname + at resources.sql_warehouses.rname.name in databricks.yml:7:7 { @@ -193,8 +180,8 @@ Error: sql_warehouse name is required } === resources.secret_scopes.rname === -Warning: required field "name" is not set - at resources.secret_scopes.rname +Error: secret_scope name is required + at resources.secret_scopes.rname.name in databricks.yml:7:7 { @@ -204,24 +191,8 @@ Warning: required field "name" is not set } === resources.alerts.rname === -Warning: required field "display_name" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "evaluation" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "query_text" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "schedule" is not set - at resources.alerts.rname - in databricks.yml:7:7 - -Warning: required field "warehouse_id" is not set - at resources.alerts.rname +Error: alert display_name is required + at resources.alerts.rname.display_name in databricks.yml:7:7 { diff --git a/acceptance/bundle/validate/invalid_identifiers/databricks.yml b/acceptance/bundle/validate/invalid_identifiers/databricks.yml new file mode 100644 index 00000000000..73a6f8a65be --- /dev/null +++ b/acceptance/bundle/validate/invalid_identifiers/databricks.yml @@ -0,0 +1,49 @@ +bundle: + name: invalid-identifiers + +resources: + models: + empty_model: + name: "" + + catalogs: + empty_catalog: + name: "" + + volumes: + tab_volume: + name: "tab\there" + catalog_name: main + schema_name: default + volume_type: MANAGED + + tab_catalog: + name: valid + catalog_name: "main\tcatalog" + schema_name: default + volume_type: MANAGED + + empty_catalog_name: + name: valid + catalog_name: "" + schema_name: default + volume_type: MANAGED + + model_serving_endpoints: + newline_endpoint: + name: "line1\nline2" + + vector_search_endpoints: + tab_endpoint: + name: "bad\tname" + endpoint_type: STANDARD + + experiments: + blank_experiment: + name: " " + + pipelines: + incomplete_file: + name: incomplete-file + libraries: + - file: {} diff --git a/acceptance/bundle/resources/models/empty-name/out.test.toml b/acceptance/bundle/validate/invalid_identifiers/out.test.toml similarity index 81% rename from acceptance/bundle/resources/models/empty-name/out.test.toml rename to acceptance/bundle/validate/invalid_identifiers/out.test.toml index 2a13818c13f..98ea5040486 100644 --- a/acceptance/bundle/resources/models/empty-name/out.test.toml +++ b/acceptance/bundle/validate/invalid_identifiers/out.test.toml @@ -1,2 +1,2 @@ -Cloud = true +Cloud = false EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/validate/invalid_identifiers/output.txt b/acceptance/bundle/validate/invalid_identifiers/output.txt new file mode 100644 index 00000000000..bd3f263b3cb --- /dev/null +++ b/acceptance/bundle/validate/invalid_identifiers/output.txt @@ -0,0 +1,53 @@ + +>>> musterr [CLI] bundle validate +Error: catalog name is required + at resources.catalogs.empty_catalog.name + in databricks.yml:11:13 + +Error: experiment name must not be blank + at resources.experiments.blank_experiment.name + in databricks.yml:43:13 + +Error: model name is required + at resources.models.empty_model.name + in databricks.yml:7:13 + +Error: model_serving_endpoint name must not contain control characters + at resources.model_serving_endpoints.newline_endpoint.name + in databricks.yml:34:13 + +The value contains U+000A at byte offset 5 + +Error: vector_search_endpoint name must not contain control characters + at resources.vector_search_endpoints.tab_endpoint.name + in databricks.yml:38:13 + +The value contains U+0009 at byte offset 3 + +Error: volume catalog_name is required + at resources.volumes.empty_catalog_name.catalog_name + in databricks.yml:28:21 + +Error: volume catalog_name must not contain control characters + at resources.volumes.tab_catalog.catalog_name + in databricks.yml:22:21 + +The value contains U+0009 at byte offset 4 + +Error: volume name must not contain control characters + at resources.volumes.tab_volume.name + in databricks.yml:15:13 + +The value contains U+0009 at byte offset 3 + +Error: pipeline library file path is required + at resources.pipelines.incomplete_file.libraries[0].file + in databricks.yml:49:17 + +Name: invalid-identifiers +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/invalid-identifiers/default + +Found 9 errors diff --git a/acceptance/bundle/validate/invalid_identifiers/script b/acceptance/bundle/validate/invalid_identifiers/script new file mode 100644 index 00000000000..3d3b920843f --- /dev/null +++ b/acceptance/bundle/validate/invalid_identifiers/script @@ -0,0 +1,2 @@ +# Catch invalid values before deployment reaches the API. +trace musterr $CLI bundle validate diff --git a/acceptance/bundle/validate/models/missing_name/output.txt b/acceptance/bundle/validate/models/missing_name/output.txt index 24e6c1d2ca5..dcbbaa92615 100644 --- a/acceptance/bundle/validate/models/missing_name/output.txt +++ b/acceptance/bundle/validate/models/missing_name/output.txt @@ -1,5 +1,5 @@ -Warning: required field "name" is not set - at resources.models.mymodel +Error: model name is required + at resources.models.mymodel.name in databricks.yml:6:14 Name: test-bundle @@ -8,4 +8,6 @@ Workspace: User: [USERNAME] Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default -Found 1 warning +Found 1 error + +Exit code: 1 diff --git a/acceptance/bundle/validate/models/user_id/databricks.yml b/acceptance/bundle/validate/models/user_id/databricks.yml index 61fc1746331..d34c78144bd 100644 --- a/acceptance/bundle/validate/models/user_id/databricks.yml +++ b/acceptance/bundle/validate/models/user_id/databricks.yml @@ -4,4 +4,5 @@ bundle: resources: models: mymodel: + name: mymodel user_id: 123 diff --git a/acceptance/bundle/validate/models/user_id/output.txt b/acceptance/bundle/validate/models/user_id/output.txt index 95c7ddde0e5..ec9c10f52e3 100644 --- a/acceptance/bundle/validate/models/user_id/output.txt +++ b/acceptance/bundle/validate/models/user_id/output.txt @@ -1,10 +1,6 @@ Warning: unknown field: user_id at resources.models.mymodel - in databricks.yml:7:7 - -Warning: required field "name" is not set - at resources.models.mymodel - in databricks.yml:7:7 + in databricks.yml:8:7 Name: test-bundle Target: default @@ -12,4 +8,4 @@ Workspace: User: [USERNAME] Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default -Found 2 warnings +Found 1 warning diff --git a/acceptance/bundle/validate/required/databricks.yml b/acceptance/bundle/validate/required/databricks.yml index 82175fb553c..41d22c9bdda 100644 --- a/acceptance/bundle/validate/required/databricks.yml +++ b/acceptance/bundle/validate/required/databricks.yml @@ -7,15 +7,6 @@ artifacts: - {} resources: - # Required field name is missing. - models: - my_model_1: - description: "hello" - - # Empty string should not trigger a warning. - my_model_2: - name: "" - jobs: my_job_1: tasks: @@ -29,7 +20,6 @@ resources: run_job_task: # Catalog name and schema name are required. - # but are not set. volumes: my_volume: name: "baz" diff --git a/acceptance/bundle/validate/required/output.txt b/acceptance/bundle/validate/required/output.txt index ae3e6dad2f8..13fe5147375 100644 --- a/acceptance/bundle/validate/required/output.txt +++ b/acceptance/bundle/validate/required/output.txt @@ -2,19 +2,15 @@ >>> [CLI] bundle validate Warning: required field "catalog_name" is not set at resources.volumes.my_volume - in databricks.yml:35:7 + in databricks.yml:25:7 Warning: required field "job_id" is not set at resources.jobs.my_job_1.tasks[1].run_job_task - in databricks.yml:29:24 - -Warning: required field "name" is not set - at resources.models.my_model_1 - in databricks.yml:13:7 + in databricks.yml:20:24 Warning: required field "schema_name" is not set at resources.volumes.my_volume - in databricks.yml:35:7 + in databricks.yml:25:7 Warning: required field "source" is not set at artifacts.my_artifact.files[0] @@ -26,4 +22,4 @@ Workspace: User: [USERNAME] Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default -Found 5 warnings +Found 4 warnings diff --git a/acceptance/bundle/validate/sql_warehouse_required_name/output.txt b/acceptance/bundle/validate/sql_warehouse_required_name/output.txt index fc66fed24e1..4bba9b58c00 100644 --- a/acceptance/bundle/validate/sql_warehouse_required_name/output.txt +++ b/acceptance/bundle/validate/sql_warehouse_required_name/output.txt @@ -1,13 +1,13 @@ >>> [CLI] bundle validate Error: sql_warehouse name is required - at resources.sql_warehouses.blank_warehouse - in databricks.yml:11:7 - -Error: sql_warehouse name is required - at resources.sql_warehouses.my_warehouse + at resources.sql_warehouses.my_warehouse.name in databricks.yml:8:7 +Error: sql_warehouse name must not be blank + at resources.sql_warehouses.blank_warehouse.name + in databricks.yml:11:13 + Name: test-bundle Target: default Workspace: diff --git a/acceptance/bundle/validate/volume_defaults/databricks.yml b/acceptance/bundle/validate/volume_defaults/databricks.yml index 159db5c7511..9fb56a3a966 100644 --- a/acceptance/bundle/validate/volume_defaults/databricks.yml +++ b/acceptance/bundle/validate/volume_defaults/databricks.yml @@ -4,10 +4,19 @@ bundle: resources: volumes: v1: + name: v1 + catalog_name: main + schema_name: default volume_type: "" v2: + name: v2 + catalog_name: main + schema_name: default volume_type: "already-set" v3: + name: v3 + catalog_name: main + schema_name: default comment: hello diff --git a/acceptance/bundle/validate/volume_defaults/output.txt b/acceptance/bundle/validate/volume_defaults/output.txt index ecdbd7ae7b7..d92703ffb69 100644 --- a/acceptance/bundle/validate/volume_defaults/output.txt +++ b/acceptance/bundle/validate/volume_defaults/output.txt @@ -1,59 +1,32 @@ -Warning: required field "catalog_name" is not set - at resources.volumes.v2 - in databricks.yml:10:7 - -Warning: required field "catalog_name" is not set - at resources.volumes.v3 - in databricks.yml:13:7 - -Warning: required field "catalog_name" is not set - at resources.volumes.v1 - in databricks.yml:7:7 - -Warning: required field "name" is not set - at resources.volumes.v2 - in databricks.yml:10:7 - -Warning: required field "name" is not set - at resources.volumes.v3 - in databricks.yml:13:7 - -Warning: required field "name" is not set - at resources.volumes.v1 - in databricks.yml:7:7 - -Warning: required field "schema_name" is not set - at resources.volumes.v2 - in databricks.yml:10:7 - -Warning: required field "schema_name" is not set - at resources.volumes.v3 - in databricks.yml:13:7 - -Warning: required field "schema_name" is not set - at resources.volumes.v1 - in databricks.yml:7:7 - Warning: invalid value "" for enum field. Valid values are [EXTERNAL MANAGED] at resources.volumes.v1.volume_type - in databricks.yml:7:20 + in databricks.yml:10:20 Warning: invalid value "already-set" for enum field. Valid values are [EXTERNAL MANAGED] at resources.volumes.v2.volume_type - in databricks.yml:10:20 + in databricks.yml:16:20 { "v1": { - "volume_path": "/Volumes///", + "catalog_name": "main", + "name": "v1", + "schema_name": "default", + "volume_path": "/Volumes/main/default/v1", "volume_type": "" }, "v2": { - "volume_path": "/Volumes///", + "catalog_name": "main", + "name": "v2", + "schema_name": "default", + "volume_path": "/Volumes/main/default/v2", "volume_type": "already-set" }, "v3": { + "catalog_name": "main", "comment": "hello", - "volume_path": "/Volumes///", + "name": "v3", + "schema_name": "default", + "volume_path": "/Volumes/main/default/v3", "volume_type": "MANAGED" } } diff --git a/bundle/config/validate/invalid_identifiers.go b/bundle/config/validate/invalid_identifiers.go new file mode 100644 index 00000000000..53480a82821 --- /dev/null +++ b/bundle/config/validate/invalid_identifiers.go @@ -0,0 +1,174 @@ +package validate + +import ( + "context" + "fmt" + "strings" + "unicode" + "unicode/utf8" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +// validateIdentifiers rejects values that the backend cannot use as identifiers. +func validateIdentifiers(_ context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + bundlePath := dyn.NewPath(dyn.Key("bundle")) + if pathExists(b, bundlePath) { + diags = diags.Extend(validateIdentifier(b, bundlePath, "name", b.Config.Bundle.Name, true)) + } + + for key, resource := range b.Config.Resources.Alerts { + diags = diags.Extend(validateResourceIdentifier(b, "alerts", key, "display_name", resource.DisplayName, true)) + } + for key, resource := range b.Config.Resources.Apps { + diags = diags.Extend(validateResourceIdentifier(b, "apps", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.Catalogs { + diags = diags.Extend(validateResourceIdentifier(b, "catalogs", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.Dashboards { + diags = diags.Extend(validateResourceIdentifier(b, "dashboards", key, "display_name", resource.DisplayName, true)) + } + for key, resource := range b.Config.Resources.DatabaseCatalogs { + diags = diags.Extend(validateResourceIdentifier(b, "database_catalogs", key, "database_instance_name", resource.DatabaseInstanceName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "database_catalogs", key, "database_name", resource.DatabaseName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "database_catalogs", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.DatabaseInstances { + diags = diags.Extend(validateResourceIdentifier(b, "database_instances", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.Experiments { + diags = diags.Extend(validateResourceIdentifier(b, "experiments", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.ExternalLocations { + diags = diags.Extend(validateResourceIdentifier(b, "external_locations", key, "credential_name", resource.CredentialName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "external_locations", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.InstancePools { + diags = diags.Extend(validateResourceIdentifier(b, "instance_pools", key, "instance_pool_name", resource.InstancePoolName, true)) + } + for key, resource := range b.Config.Resources.ModelServingEndpoints { + diags = diags.Extend(validateResourceIdentifier(b, "model_serving_endpoints", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.Models { + diags = diags.Extend(validateResourceIdentifier(b, "models", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.QualityMonitors { + diags = diags.Extend(validateResourceIdentifier(b, "quality_monitors", key, "output_schema_name", resource.OutputSchemaName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "quality_monitors", key, "table_name", resource.TableName, false)) + } + for key, resource := range b.Config.Resources.RegisteredModels { + diags = diags.Extend(validateResourceIdentifier(b, "registered_models", key, "catalog_name", resource.CatalogName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "registered_models", key, "name", resource.Name, true)) + diags = diags.Extend(validateResourceIdentifier(b, "registered_models", key, "schema_name", resource.SchemaName, false)) + } + for key, resource := range b.Config.Resources.Schemas { + diags = diags.Extend(validateResourceIdentifier(b, "schemas", key, "catalog_name", resource.CatalogName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "schemas", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.Secrets { + diags = diags.Extend(validateResourceIdentifier(b, "secrets", key, "catalog_name", resource.CatalogName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "secrets", key, "name", resource.Name, true)) + diags = diags.Extend(validateResourceIdentifier(b, "secrets", key, "schema_name", resource.SchemaName, false)) + } + for key, resource := range b.Config.Resources.SecretScopes { + diags = diags.Extend(validateResourceIdentifier(b, "secret_scopes", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.SqlWarehouses { + diags = diags.Extend(validateResourceIdentifier(b, "sql_warehouses", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.SyncedDatabaseTables { + diags = diags.Extend(validateResourceIdentifier(b, "synced_database_tables", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.VectorSearchEndpoints { + diags = diags.Extend(validateResourceIdentifier(b, "vector_search_endpoints", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.VectorSearchIndexes { + diags = diags.Extend(validateResourceIdentifier(b, "vector_search_indexes", key, "endpoint_name", resource.EndpointName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "vector_search_indexes", key, "name", resource.Name, true)) + } + for key, resource := range b.Config.Resources.Volumes { + diags = diags.Extend(validateResourceIdentifier(b, "volumes", key, "catalog_name", resource.CatalogName, false)) + diags = diags.Extend(validateResourceIdentifier(b, "volumes", key, "name", resource.Name, true)) + diags = diags.Extend(validateResourceIdentifier(b, "volumes", key, "schema_name", resource.SchemaName, false)) + } + + sortDiagnostics(diags) + return diags +} + +func validateResourceIdentifier(b *bundle.Bundle, resourceType, key, field, value string, required bool) diag.Diagnostics { + resourcePath := dyn.NewPath( + dyn.Key("resources"), + dyn.Key(resourceType), + dyn.Key(key), + ) + return validateIdentifier(b, resourcePath, field, value, required) +} + +func validateIdentifier(b *bundle.Bundle, resourcePath dyn.Path, field, value string, required bool) diag.Diagnostics { + fieldPath := resourcePath.Append(dyn.Key(field)) + locations := locationsAtPath(b, fieldPath) + if value == "" && !required && !pathExists(b, fieldPath) { + return nil + } + + reason, detail := invalidIdentifierReason(value) + if reason == "" { + return nil + } + if len(locations) == 0 { + locations = locationsAtPath(b, resourcePath) + } + return diag.Diagnostics{{ + Severity: diag.Error, + Summary: fmt.Sprintf("%s %s %s", requiredObjectName(resourcePath), field, reason), + Detail: detail, + Locations: locations, + Paths: []dyn.Path{fieldPath}, + }} +} + +// locationsAtPath avoids GetLocations: string paths do not round-trip keys with '[' or '.'. +func locationsAtPath(b *bundle.Bundle, path dyn.Path) []dyn.Location { + value, err := dyn.GetByPath(b.Config.Value(), path) + if err != nil { + return nil + } + return value.Locations() +} + +func pathExists(b *bundle.Bundle, path dyn.Path) bool { + _, err := dyn.GetByPath(b.Config.Value(), path) + return err == nil +} + +func invalidIdentifierReason(value string) (reason, detail string) { + if value == "" { + return "is required", "" + } + if i := strings.IndexFunc(value, unicode.IsControl); i >= 0 { + r, _ := utf8.DecodeRuneInString(value[i:]) + return "must not contain control characters", fmt.Sprintf("The value contains %U at byte offset %d", r, i) + } + if strings.TrimSpace(value) == "" { + return "must not be blank", "" + } + return "", "" +} + +func requiredObjectName(path dyn.Path) string { + if len(path) == 1 { + return path[0].Key() + } + plural := path[1].Key() + if desc, ok := config.SupportedResources()[plural]; ok && desc.SingularName != "" { + return desc.SingularName + } + return plural +} diff --git a/bundle/config/validate/invalid_identifiers_test.go b/bundle/config/validate/invalid_identifiers_test.go new file mode 100644 index 00000000000..9bdc94226be --- /dev/null +++ b/bundle/config/validate/invalid_identifiers_test.go @@ -0,0 +1,276 @@ +package validate_test + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/config/validate" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/catalog" + "github.com/databricks/databricks-sdk-go/service/ml" + "github.com/databricks/databricks-sdk-go/service/serving" + "github.com/databricks/databricks-sdk-go/service/vectorsearch" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRequiredRejectsEmptyAndControlCharIdentifiers(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Models: map[string]*resources.MlflowModel{ + "empty": {CreateModelRequest: ml.CreateModelRequest{Name: ""}}, + }, + Catalogs: map[string]*resources.Catalog{ + "blank": {CreateCatalog: catalog.CreateCatalog{Name: " "}}, + }, + Volumes: map[string]*resources.Volume{ + "ctrl": { + CreateVolumeRequestContent: catalog.CreateVolumeRequestContent{ + Name: "tab\there", + CatalogName: "main", + SchemaName: "default", + VolumeType: catalog.VolumeTypeManaged, + }, + }, + "empty_parent": { + CreateVolumeRequestContent: catalog.CreateVolumeRequestContent{ + Name: "v", + CatalogName: "", + SchemaName: "default", + VolumeType: catalog.VolumeTypeManaged, + }, + }, + }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "nl": { + CreateServingEndpoint: serving.CreateServingEndpoint{ + Name: "line1\nline2", + }, + }, + }, + VectorSearchEndpoints: map[string]*resources.VectorSearchEndpoint{ + "vse": { + CreateEndpoint: vectorsearch.CreateEndpoint{ + Name: "bad\tname", + EndpointType: vectorsearch.EndpointTypeStandard, + }, + }, + }, + Experiments: map[string]*resources.MlflowExperiment{ + "e": {CreateExperiment: ml.CreateExperiment{Name: ""}}, + }, + }, + }, + } + + diags := bundle.Apply(t.Context(), b, validate.Required()) + require.True(t, diags.HasError()) + + assert.ElementsMatch(t, []string{ + "catalog name must not be blank", + "model name is required", + "volume name must not contain control characters", + "model_serving_endpoint name must not contain control characters", + "vector_search_endpoint name must not contain control characters", + "experiment name is required", + }, diagSummaries(diags)) +} + +func TestRequiredRejectsExplicitEmptyUCParentInDyn(t *testing.T) { + b := &bundle.Bundle{} + require.NoError(t, b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.V(map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + "volumes": dyn.V(map[string]dyn.Value{ + "v": dyn.V(map[string]dyn.Value{ + "name": dyn.V("v"), + "catalog_name": dyn.V(""), + "schema_name": dyn.V("default"), + "volume_type": dyn.V("MANAGED"), + }), + }), + }), + }), nil + })) + + diags := bundle.Apply(t.Context(), b, validate.Required()) + require.True(t, diags.HasError()) + assert.Contains(t, diagSummaries(diags), "volume catalog_name is required") +} + +func TestRequiredIdentifierValidationScope(t *testing.T) { + b := &bundle.Bundle{} + require.NoError(t, b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.V(map[string]dyn.Value{ + "bundle": dyn.V(map[string]dyn.Value{ + "name": dyn.V(" \t"), + }), + "resources": dyn.V(map[string]dyn.Value{ + "dashboards": dyn.V(map[string]dyn.Value{ + "dashboard": dyn.V(map[string]dyn.Value{ + "display_name": dyn.V("bad\nname"), + "warehouse_id": dyn.V("warehouse"), + }), + }), + "jobs": dyn.V(map[string]dyn.Value{ + "job": dyn.V(map[string]dyn.Value{ + "parameters": dyn.V([]dyn.Value{ + dyn.V(map[string]dyn.Value{"default": dyn.V("value")}), + }), + }), + }), + "sql_warehouses": dyn.V(map[string]dyn.Value{ + "warehouse": dyn.V(map[string]dyn.Value{ + "name": dyn.V(" "), + }), + }), + }), + }), nil + })) + + diags := bundle.Apply(t.Context(), b, validate.Required()) + assert.ElementsMatch(t, []string{ + "bundle name must not contain control characters", + "dashboard display_name must not contain control characters", + "sql_warehouse name must not be blank", + }, diagSummaries(diags)) +} + +func TestRequiredPreservesLocationForMetacharacterResourceKey(t *testing.T) { + location := dyn.Location{File: "databricks.yml", Line: 6, Column: 13} + b := &bundle.Bundle{} + require.NoError(t, b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.V(map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + "volumes": dyn.V(map[string]dyn.Value{ + "weird[0]key": dyn.V(map[string]dyn.Value{ + "name": dyn.NewValue("", []dyn.Location{location}), + "catalog_name": dyn.V("main"), + "schema_name": dyn.V("default"), + "volume_type": dyn.V("MANAGED"), + }), + }), + }), + }), nil + })) + + diags := bundle.Apply(t.Context(), b, validate.Required()) + require.True(t, diags.HasError()) + require.Len(t, diags, 1) + assert.Equal(t, "volume name is required", diags[0].Summary) + assert.Equal(t, []dyn.Location{location}, diags[0].Locations) +} + +func TestRequiredAcceptsValidIdentifiers(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Models: map[string]*resources.MlflowModel{ + "model": {CreateModelRequest: ml.CreateModelRequest{Name: "model"}}, + }, + }, + }, + } + + assert.Empty(t, bundle.Apply(t.Context(), b, validate.Required())) +} + +func TestRequiredAcceptsMissingOptionalUCParents(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + RegisteredModels: map[string]*resources.RegisteredModel{ + "model": { + CreateRegisteredModelRequest: catalog.CreateRegisteredModelRequest{ + Name: "model", + }, + }, + }, + }, + }, + } + + assert.Empty(t, bundle.Apply(t.Context(), b, validate.Required())) +} + +func TestRequiredRejectsBlankOptionalUCParent(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + RegisteredModels: map[string]*resources.RegisteredModel{ + "model": { + CreateRegisteredModelRequest: catalog.CreateRegisteredModelRequest{ + Name: "model", + CatalogName: " ", + }, + }, + }, + }, + }, + } + + diags := bundle.Apply(t.Context(), b, validate.Required()) + require.True(t, diags.HasError()) + assert.Contains(t, diagSummaries(diags), "registered_model catalog_name must not be blank") +} + +func TestRequiredRejectsMissingResourceIdentifiers(t *testing.T) { + tests := []struct { + resourceType string + summary string + }{ + {"alerts", "alert display_name is required"}, + {"apps", "app name is required"}, + {"catalogs", "catalog name is required"}, + {"dashboards", "dashboard display_name is required"}, + {"database_catalogs", "database_catalog name is required"}, + {"database_instances", "database_instance name is required"}, + {"experiments", "experiment name is required"}, + {"external_locations", "external_location name is required"}, + {"instance_pools", "instance_pool instance_pool_name is required"}, + {"model_serving_endpoints", "model_serving_endpoint name is required"}, + {"models", "model name is required"}, + {"registered_models", "registered_model name is required"}, + {"schemas", "schema name is required"}, + {"secret_scopes", "secret_scope name is required"}, + {"secrets", "secret name is required"}, + {"sql_warehouses", "sql_warehouse name is required"}, + {"synced_database_tables", "synced_database_table name is required"}, + {"vector_search_endpoints", "vector_search_endpoint name is required"}, + {"vector_search_indexes", "vector_search_index name is required"}, + {"volumes", "volume name is required"}, + } + + for _, tt := range tests { + t.Run(tt.resourceType, func(t *testing.T) { + b := &bundle.Bundle{} + require.NoError(t, b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.V(map[string]dyn.Value{ + "bundle": dyn.V(map[string]dyn.Value{ + "name": dyn.V("bundle"), + }), + "resources": dyn.V(map[string]dyn.Value{ + tt.resourceType: dyn.V(map[string]dyn.Value{ + "weird[0]key": dyn.V(map[string]dyn.Value{}), + }), + }), + }), nil + })) + + diags := bundle.Apply(t.Context(), b, validate.Required()) + assert.Contains(t, diagSummaries(diags), tt.summary) + }) + } +} + +func diagSummaries(diags diag.Diagnostics) []string { + out := make([]string, 0, len(diags)) + for _, d := range diags { + out = append(out, d.Summary) + } + return out +} diff --git a/bundle/config/validate/pipeline_libraries.go b/bundle/config/validate/pipeline_libraries.go new file mode 100644 index 00000000000..d09440b07a5 --- /dev/null +++ b/bundle/config/validate/pipeline_libraries.go @@ -0,0 +1,52 @@ +package validate + +import ( + "context" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +// errorForIncompletePipelineLibraries rejects file, notebook, and glob entries +// without paths because the pipelines API rejects them. +func errorForIncompletePipelineLibraries(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + for key, pipeline := range b.Config.Resources.Pipelines { + for i, library := range pipeline.Libraries { + base := dyn.NewPath( + dyn.Key("resources"), + dyn.Key("pipelines"), + dyn.Key(key), + dyn.Key("libraries"), + dyn.Index(i), + ) + if library.File != nil && strings.TrimSpace(library.File.Path) == "" { + diags = diags.Append(libraryPathDiag(b, base, "file", "pipeline library file path is required")) + } + if library.Notebook != nil && strings.TrimSpace(library.Notebook.Path) == "" { + diags = diags.Append(libraryPathDiag(b, base, "notebook", "pipeline library notebook path is required")) + } + if library.Glob != nil && strings.TrimSpace(library.Glob.Include) == "" { + diags = diags.Append(libraryPathDiag(b, base, "glob", "pipeline library glob include is required")) + } + } + } + return diags +} + +// libraryPathDiag reports a missing path on one pipeline library variant. +func libraryPathDiag(b *bundle.Bundle, base dyn.Path, field, summary string) diag.Diagnostic { + fieldPath := base.Append(dyn.Key(field)) + locations := locationsAtPath(b, fieldPath) + if len(locations) == 0 { + locations = locationsAtPath(b, base) + } + return diag.Diagnostic{ + Severity: diag.Error, + Summary: summary, + Locations: locations, + Paths: []dyn.Path{fieldPath}, + } +} diff --git a/bundle/config/validate/pipeline_libraries_test.go b/bundle/config/validate/pipeline_libraries_test.go new file mode 100644 index 00000000000..9953404e53d --- /dev/null +++ b/bundle/config/validate/pipeline_libraries_test.go @@ -0,0 +1,92 @@ +package validate_test + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/config/validate" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/pipelines" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRequiredRejectsIncompletePipelineLibraries(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Pipelines: map[string]*resources.Pipeline{ + "weird[0]key": { + CreatePipeline: pipelines.CreatePipeline{ + Name: "p", + Libraries: []pipelines.PipelineLibrary{ + {File: &pipelines.FileLibrary{}}, + {Notebook: &pipelines.NotebookLibrary{}}, + {Glob: &pipelines.PathPattern{}}, + {File: &pipelines.FileLibrary{Path: "ok.py"}}, + }, + }, + }, + }, + }, + }, + } + + diags := bundle.Apply(t.Context(), b, validate.Required()) + require.True(t, diags.HasError()) + assert.ElementsMatch(t, []string{ + "pipeline library file path is required", + "pipeline library notebook path is required", + "pipeline library glob include is required", + }, diagSummaries(diags)) +} + +func TestRequiredPreservesPipelineLibraryLocationForMetacharacterResourceKey(t *testing.T) { + location := dyn.Location{File: "databricks.yml", Line: 8, Column: 17} + b := &bundle.Bundle{} + require.NoError(t, b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.V(map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + "pipelines": dyn.V(map[string]dyn.Value{ + "weird[0]key": dyn.V(map[string]dyn.Value{ + "libraries": dyn.V([]dyn.Value{ + dyn.V(map[string]dyn.Value{ + "file": dyn.NewValue(map[string]dyn.Value{}, []dyn.Location{location}), + }), + }), + }), + }), + }), + }), nil + })) + + diags := bundle.Apply(t.Context(), b, validate.Required()) + require.Len(t, diags, 1) + assert.Equal(t, "pipeline library file path is required", diags[0].Summary) + assert.Equal(t, []dyn.Location{location}, diags[0].Locations) +} + +func TestRequiredAcceptsCompletePipelineLibraries(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Pipelines: map[string]*resources.Pipeline{ + "pipeline": { + CreatePipeline: pipelines.CreatePipeline{ + Name: "pipeline", + Libraries: []pipelines.PipelineLibrary{ + {File: &pipelines.FileLibrary{Path: "file.py"}}, + {Notebook: &pipelines.NotebookLibrary{Path: "notebook.py"}}, + {Glob: &pipelines.PathPattern{Include: "src/**"}}, + }, + }, + }, + }, + }, + }, + } + + assert.Empty(t, bundle.Apply(t.Context(), b, validate.Required())) +} diff --git a/bundle/config/validate/required.go b/bundle/config/validate/required.go index b886c2c1d73..7541167f59c 100644 --- a/bundle/config/validate/required.go +++ b/bundle/config/validate/required.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "slices" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/internal/validation/generated" @@ -95,32 +94,23 @@ func sortDiagnostics(diags diag.Diagnostics) { // Bespoke code to error for fields that are not marked as required in the Go SDK / OpenAPI spec. func errorForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - // Dashboards should always have a name and warehouse_id. - var nameLocations []dyn.Location - var namePaths []dyn.Path + // Dashboards should always have a warehouse_id. var warehouseIdLocations []dyn.Location var warehouseIdPaths []dyn.Path diags := diag.Diagnostics{} for key, dashboard := range b.Config.Resources.Dashboards { - if dashboard.DisplayName == "" { - nameLocations = append(nameLocations, b.Config.GetLocations("resources.dashboards."+key)...) - namePaths = append(namePaths, dyn.MustPathFromString("resources.dashboards."+key)) - } if dashboard.WarehouseId == "" { - warehouseIdLocations = append(warehouseIdLocations, b.Config.GetLocations("resources.dashboards."+key)...) - warehouseIdPaths = append(warehouseIdPaths, dyn.MustPathFromString("resources.dashboards."+key)) + resourcePath := dyn.NewPath( + dyn.Key("resources"), + dyn.Key("dashboards"), + dyn.Key(key), + ) + warehouseIdLocations = append(warehouseIdLocations, locationsAtPath(b, resourcePath)...) + warehouseIdPaths = append(warehouseIdPaths, resourcePath) } } - if len(nameLocations) > 0 { - diags = diags.Append(diag.Diagnostic{ - Severity: diag.Error, - Summary: "dashboard display_name is required", - Locations: nameLocations, - Paths: namePaths, - }) - } if len(warehouseIdLocations) > 0 { diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, @@ -130,20 +120,6 @@ func errorForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnosti }) } - // sql_warehouses.name is optional in the SDK (json:"name,omitempty") but required - // by the backend, which rejects whitespace-only names (name.trim.nonEmpty). - for key, warehouse := range b.Config.Resources.SqlWarehouses { - if strings.TrimSpace(warehouse.Name) == "" { - path := "resources.sql_warehouses." + key - diags = diags.Append(diag.Diagnostic{ - Severity: diag.Error, - Summary: "sql_warehouse name is required", - Locations: b.Config.GetLocations(path), - Paths: []dyn.Path{dyn.MustPathFromString(path)}, - }) - } - } - sortDiagnostics(diags) return diags @@ -242,9 +218,11 @@ func isMissingOrEmptySequence(v dyn.Value) bool { } func (f *required) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - diags := errorForMissingFields(ctx, b) + diags := validateIdentifiers(ctx, b) + diags = diags.Extend(errorForMissingFields(ctx, b)) diags = diags.Extend(errorForInvalidGrants(ctx, b)) diags = diags.Extend(errorForInvalidSecretScopePermissions(ctx, b)) + diags = diags.Extend(errorForIncompletePipelineLibraries(ctx, b)) if diags.HasError() { return diags }