diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 57d93cb0fcd..e3f94f8035b 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -11,6 +11,7 @@ ### Bundles * `bundle generate job` now downloads workspace files referenced by `spark_python_task`, rewriting them to a relative path like it already does for notebooks. Git-sourced files and cloud URIs are left untouched ([#5799](https://github.com/databricks/cli/pull/5799)). + * `bundle validate` now fails early with a clear error when a `sql_warehouse` is missing a `name`, a grant is missing a `principal`, or a catalog/schema `custom_max_retention_hours` is outside the allowed range (0 or 168-720 hours), instead of passing validation and failing later at deploy. ### Dependency updates diff --git a/acceptance/bundle/validate/empty_resources/empty_dict/output.txt b/acceptance/bundle/validate/empty_resources/empty_dict/output.txt index efbf60ca1ae..c31d1773e88 100644 --- a/acceptance/bundle/validate/empty_resources/empty_dict/output.txt +++ b/acceptance/bundle/validate/empty_resources/empty_dict/output.txt @@ -160,6 +160,10 @@ 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 + in databricks.yml:6:12 + { "sql_warehouses": { "rname": { diff --git a/acceptance/bundle/validate/empty_resources/with_grants/output.txt b/acceptance/bundle/validate/empty_resources/with_grants/output.txt index e9d371b33d5..2c7e4bd5217 100644 --- a/acceptance/bundle/validate/empty_resources/with_grants/output.txt +++ b/acceptance/bundle/validate/empty_resources/with_grants/output.txt @@ -201,6 +201,10 @@ Warning: unknown field: grants at resources.sql_warehouses.rname in databricks.yml:7:7 +Error: sql_warehouse name is required + at resources.sql_warehouses.rname + in databricks.yml:7:7 + { "sql_warehouses": { "rname": { diff --git a/acceptance/bundle/validate/empty_resources/with_permissions/output.txt b/acceptance/bundle/validate/empty_resources/with_permissions/output.txt index 28957886423..ba521d61c97 100644 --- a/acceptance/bundle/validate/empty_resources/with_permissions/output.txt +++ b/acceptance/bundle/validate/empty_resources/with_permissions/output.txt @@ -176,6 +176,10 @@ 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 + in databricks.yml:7:7 + { "sql_warehouses": { "rname": { diff --git a/acceptance/bundle/validate/grants_required_principal/databricks.yml b/acceptance/bundle/validate/grants_required_principal/databricks.yml new file mode 100644 index 00000000000..4e81d9cb165 --- /dev/null +++ b/acceptance/bundle/validate/grants_required_principal/databricks.yml @@ -0,0 +1,21 @@ +bundle: + name: test-bundle + +resources: + catalogs: + my_catalog: + name: my_catalog + grants: + # Missing principal: required by the backend, optional in the SDK + # (json:"principal,omitempty"). + - privileges: + - ALL_PRIVILEGES + schemas: + my_schema: + catalog_name: my_catalog + name: my_schema + grants: + # Valid grant: principal is set. + - principal: alice@example.com + privileges: + - USE_SCHEMA diff --git a/acceptance/bundle/validate/grants_required_principal/out.test.toml b/acceptance/bundle/validate/grants_required_principal/out.test.toml new file mode 100644 index 00000000000..e90b6d5d1ba --- /dev/null +++ b/acceptance/bundle/validate/grants_required_principal/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/validate/grants_required_principal/output.txt b/acceptance/bundle/validate/grants_required_principal/output.txt new file mode 100644 index 00000000000..f40aab1bab7 --- /dev/null +++ b/acceptance/bundle/validate/grants_required_principal/output.txt @@ -0,0 +1,15 @@ + +>>> [CLI] bundle validate +Error: grant principal is required + at resources.catalogs.my_catalog.grants[0] + in databricks.yml:11:11 + +Name: test-bundle +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Found 1 error + +Exit code: 1 diff --git a/acceptance/bundle/validate/grants_required_principal/script b/acceptance/bundle/validate/grants_required_principal/script new file mode 100644 index 00000000000..5350876150f --- /dev/null +++ b/acceptance/bundle/validate/grants_required_principal/script @@ -0,0 +1 @@ +trace $CLI bundle validate diff --git a/acceptance/bundle/validate/grants_required_principal/test.toml b/acceptance/bundle/validate/grants_required_principal/test.toml new file mode 100644 index 00000000000..fc2b3f50667 --- /dev/null +++ b/acceptance/bundle/validate/grants_required_principal/test.toml @@ -0,0 +1,3 @@ +# Catalogs and schemas are only supported by the direct engine. +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/validate/retention_hours_range/databricks.yml b/acceptance/bundle/validate/retention_hours_range/databricks.yml new file mode 100644 index 00000000000..b14ed9e57e0 --- /dev/null +++ b/acceptance/bundle/validate/retention_hours_range/databricks.yml @@ -0,0 +1,19 @@ +bundle: + name: test-bundle + +resources: + catalogs: + invalid_catalog: + name: invalid_catalog + # Named in hours, but the backend only accepts 0 or 168-720 (7-30 days). + custom_max_retention_hours: 7 + valid_catalog: + name: valid_catalog + # 240 hours = 10 days, within range. + custom_max_retention_hours: 240 + schemas: + invalid_schema: + catalog_name: valid_catalog + name: invalid_schema + # 1000 hours is out of range (> 30 days). + custom_max_retention_hours: 1000 diff --git a/acceptance/bundle/validate/retention_hours_range/out.test.toml b/acceptance/bundle/validate/retention_hours_range/out.test.toml new file mode 100644 index 00000000000..e90b6d5d1ba --- /dev/null +++ b/acceptance/bundle/validate/retention_hours_range/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/validate/retention_hours_range/output.txt b/acceptance/bundle/validate/retention_hours_range/output.txt new file mode 100644 index 00000000000..499951a3d13 --- /dev/null +++ b/acceptance/bundle/validate/retention_hours_range/output.txt @@ -0,0 +1,19 @@ + +>>> [CLI] bundle validate +Error: custom_max_retention_hours must be 0 or between 168 and 720 hours (7 to 30 days), got 7 + at resources.catalogs.invalid_catalog.custom_max_retention_hours + in databricks.yml:9:35 + +Error: custom_max_retention_hours must be 0 or between 168 and 720 hours (7 to 30 days), got 1000 + at resources.schemas.invalid_schema.custom_max_retention_hours + in databricks.yml:19:35 + +Name: test-bundle +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Found 2 errors + +Exit code: 1 diff --git a/acceptance/bundle/validate/retention_hours_range/script b/acceptance/bundle/validate/retention_hours_range/script new file mode 100644 index 00000000000..5350876150f --- /dev/null +++ b/acceptance/bundle/validate/retention_hours_range/script @@ -0,0 +1 @@ +trace $CLI bundle validate diff --git a/acceptance/bundle/validate/retention_hours_range/test.toml b/acceptance/bundle/validate/retention_hours_range/test.toml new file mode 100644 index 00000000000..fc2b3f50667 --- /dev/null +++ b/acceptance/bundle/validate/retention_hours_range/test.toml @@ -0,0 +1,3 @@ +# Catalogs and schemas are only supported by the direct engine. +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/validate/sql_warehouse_required_name/databricks.yml b/acceptance/bundle/validate/sql_warehouse_required_name/databricks.yml new file mode 100644 index 00000000000..e0cd3a238de --- /dev/null +++ b/acceptance/bundle/validate/sql_warehouse_required_name/databricks.yml @@ -0,0 +1,9 @@ +bundle: + name: test-bundle + +resources: + sql_warehouses: + # Missing name: required by the backend, optional in the SDK + # (json:"name,omitempty"). + my_warehouse: + cluster_size: "2X-Small" diff --git a/acceptance/bundle/validate/sql_warehouse_required_name/out.test.toml b/acceptance/bundle/validate/sql_warehouse_required_name/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/bundle/validate/sql_warehouse_required_name/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/validate/sql_warehouse_required_name/output.txt b/acceptance/bundle/validate/sql_warehouse_required_name/output.txt new file mode 100644 index 00000000000..726fcec42b5 --- /dev/null +++ b/acceptance/bundle/validate/sql_warehouse_required_name/output.txt @@ -0,0 +1,15 @@ + +>>> [CLI] bundle validate +Error: sql_warehouse name is required + at resources.sql_warehouses.my_warehouse + in databricks.yml:9:7 + +Name: test-bundle +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Found 1 error + +Exit code: 1 diff --git a/acceptance/bundle/validate/sql_warehouse_required_name/script b/acceptance/bundle/validate/sql_warehouse_required_name/script new file mode 100644 index 00000000000..5350876150f --- /dev/null +++ b/acceptance/bundle/validate/sql_warehouse_required_name/script @@ -0,0 +1 @@ +trace $CLI bundle validate diff --git a/bundle/config/validate/required.go b/bundle/config/validate/required.go index 6f886caab44..c8721046c76 100644 --- a/bundle/config/validate/required.go +++ b/bundle/config/validate/required.go @@ -14,6 +14,13 @@ import ( type required struct{} +// custom_max_retention_hours accepts 0 (disabled) or 7-30 days; other values +// fail at deploy with a low-context error, so we reject them early. +const ( + minCustomRetentionHours = 168 // 7 days + maxCustomRetentionHours = 720 // 30 days +) + func Required() bundle.Mutator { return &required{} } @@ -119,11 +126,98 @@ func errorForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnosti }) } + // sql_warehouses.name is required by the backend but optional in the SDK + // (json:"name,omitempty"), so warnForMissingFields never flags it. + _, err := dyn.MapByPattern( + b.Config.Value(), + dyn.NewPattern(dyn.Key("resources"), dyn.Key("sql_warehouses"), dyn.AnyKey()), + func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + if isMissingOrEmptyString(v.Get("name")) { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: "sql_warehouse name is required", + Locations: v.Locations(), + Paths: []dyn.Path{slices.Clone(p)}, + }) + } + return v, nil + }, + ) + if err != nil { + return diag.FromErr(err) + } + + // grants[*].principal is required by the backend but optional in the SDK + // (json:"principal,omitempty"). Grants exist on every securable, so match any. + _, err = dyn.MapByPattern( + b.Config.Value(), + dyn.NewPattern(dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey(), dyn.Key("grants"), dyn.AnyIndex()), + func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + if isMissingOrEmptyString(v.Get("principal")) { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: "grant principal is required", + Locations: v.Locations(), + Paths: []dyn.Path{slices.Clone(p)}, + }) + } + return v, nil + }, + ) + if err != nil { + return diag.FromErr(err) + } + + return diags +} + +// isMissingOrEmptyString reports whether v is unset, null, or an empty string. +func isMissingOrEmptyString(v dyn.Value) bool { + switch v.Kind() { + case dyn.KindInvalid, dyn.KindNil: + return true + case dyn.KindString: + return v.MustString() == "" + default: + return false + } +} + +// errorForInvalidRetentionHours rejects out-of-range custom_max_retention_hours on +// catalogs and schemas before deploy. +func errorForInvalidRetentionHours(b *bundle.Bundle) diag.Diagnostics { + diags := diag.Diagnostics{} + for _, section := range []string{"catalogs", "schemas"} { + _, err := dyn.MapByPattern( + b.Config.Value(), + dyn.NewPattern(dyn.Key("resources"), dyn.Key(section), dyn.AnyKey(), dyn.Key("custom_max_retention_hours")), + func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + hours, ok := v.AsInt() + if !ok { + return v, nil + } + if hours == 0 || (hours >= minCustomRetentionHours && hours <= maxCustomRetentionHours) { + return v, nil + } + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("custom_max_retention_hours must be 0 or between %d and %d hours (7 to 30 days), got %d", minCustomRetentionHours, maxCustomRetentionHours, hours), + Locations: v.Locations(), + Paths: []dyn.Path{slices.Clone(p)}, + }) + return v, nil + }, + ) + if err != nil { + diags = diags.Extend(diag.FromErr(err)) + } + } return diags } func (f *required) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags := errorForMissingFields(ctx, b) + diags = diags.Extend(errorForInvalidRetentionHours(b)) if diags.HasError() { return diags }