diff --git a/.nextchanges/bundles/empty-task-parameters-warning.md b/.nextchanges/bundles/empty-task-parameters-warning.md new file mode 100644 index 00000000000..126c811b160 --- /dev/null +++ b/.nextchanges/bundles/empty-task-parameters-warning.md @@ -0,0 +1 @@ +Warn at validate time when a task parameter is an empty string and the bundle deploys with the Terraform engine. Such deploys fail while creating the job with a provider panic (`parameters[] is not a string`); the warning names the config location before terraform runs. diff --git a/acceptance/bundle/variables/empty-in-task-params-variants/databricks.yml b/acceptance/bundle/variables/empty-in-task-params-variants/databricks.yml new file mode 100644 index 00000000000..bbf84194a7a --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params-variants/databricks.yml @@ -0,0 +1,48 @@ +bundle: + name: empty-variants + +variables: + user_prefix: + description: Optional prefix, empty everywhere but local dev + default: "" + derived_prefix: + description: Variable whose default is another variable + default: ${var.user_prefix} + settings: + description: Complex variable with an empty string field + type: complex + default: + prefix: "" + +resources: + jobs: + demo_job: + name: demo-job + tasks: + - task_key: main + spark_python_task: + python_file: ./src/main.py + parameters: + - "--a" + - ${var.user_prefix} + - "--b" + - ${var.derived_prefix} + - "--c" + - ${var.settings.prefix} + new_cluster: + spark_version: 15.4.x-scala2.12 + node_type_id: i3.xlarge + num_workers: 1 + +targets: + short_form_override: + default: true + variables: + user_prefix: "" + long_form_override: + variables: + user_prefix: + default: "" + null_override: + variables: + user_prefix: diff --git a/acceptance/bundle/variables/empty-in-task-params-variants/out.test.toml b/acceptance/bundle/variables/empty-in-task-params-variants/out.test.toml new file mode 100644 index 00000000000..1a3e24fa574 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params-variants/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false +GOOSOnPR.darwin = false +GOOSOnPR.windows = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/variables/empty-in-task-params-variants/output.txt b/acceptance/bundle/variables/empty-in-task-params-variants/output.txt new file mode 100644 index 00000000000..7bf7a98178e --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params-variants/output.txt @@ -0,0 +1,55 @@ + +=== Target with short-form empty override (targets.X.variables.user_prefix: "") +>>> [CLI] bundle validate -o json -t short_form_override +[ + "--a", + "", + "--b", + "", + "--c", + "" +] + +=== Target with long-form empty override (default: "") +>>> [CLI] bundle validate -o json -t long_form_override +[ + "--a", + "", + "--b", + "", + "--c", + "" +] + +=== Target with null override (targets.X.variables.user_prefix:) +>>> errcode [CLI] bundle validate -o json -t null_override +[ + "--a", + "", + "--b", + "", + "--c", + "" +] + +=== Empty value via BUNDLE_VAR env var +>>> env BUNDLE_VAR_user_prefix= [CLI] bundle validate -o json -t short_form_override +[ + "--a", + "", + "--b", + "", + "--c", + "" +] + +=== Empty value via --var flag +>>> [CLI] bundle validate -o json -t short_form_override --var=user_prefix= +[ + "--a", + "", + "--b", + "", + "--c", + "" +] diff --git a/acceptance/bundle/variables/empty-in-task-params-variants/script b/acceptance/bundle/variables/empty-in-task-params-variants/script new file mode 100644 index 00000000000..f2bff196380 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params-variants/script @@ -0,0 +1,14 @@ +title "Target with short-form empty override (targets.X.variables.user_prefix: \"\")" +trace $CLI bundle validate -o json -t short_form_override | jq '.resources.jobs.demo_job.tasks[0].spark_python_task.parameters' + +title "Target with long-form empty override (default: \"\")" +trace $CLI bundle validate -o json -t long_form_override | jq '.resources.jobs.demo_job.tasks[0].spark_python_task.parameters' + +title "Target with null override (targets.X.variables.user_prefix:)" +trace errcode $CLI bundle validate -o json -t null_override | jq '.resources.jobs.demo_job.tasks[0].spark_python_task.parameters' + +title "Empty value via BUNDLE_VAR env var" +trace env BUNDLE_VAR_user_prefix= $CLI bundle validate -o json -t short_form_override | jq '.resources.jobs.demo_job.tasks[0].spark_python_task.parameters' + +title "Empty value via --var flag" +trace $CLI bundle validate -o json -t short_form_override --var="user_prefix=" | jq '.resources.jobs.demo_job.tasks[0].spark_python_task.parameters' diff --git a/acceptance/bundle/variables/empty-in-task-params-variants/src/main.py b/acceptance/bundle/variables/empty-in-task-params-variants/src/main.py new file mode 100644 index 00000000000..11b15b1a458 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params-variants/src/main.py @@ -0,0 +1 @@ +print("hello") diff --git a/acceptance/bundle/variables/empty-in-task-params-variants/test.toml b/acceptance/bundle/variables/empty-in-task-params-variants/test.toml new file mode 100644 index 00000000000..9609e1af299 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params-variants/test.toml @@ -0,0 +1 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/variables/empty-in-task-params/databricks.yml b/acceptance/bundle/variables/empty-in-task-params/databricks.yml new file mode 100644 index 00000000000..ae98703a906 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/databricks.yml @@ -0,0 +1,23 @@ +bundle: + name: empty-in-task-params + +variables: + run_suffix: + description: Optional suffix appended to job parameters + default: "" + +resources: + jobs: + demo_job: + name: demo-job + tasks: + - task_key: main + spark_python_task: + python_file: ./src/main.py + parameters: + - "--suffix" + - ${var.run_suffix} + new_cluster: + spark_version: 15.4.x-scala2.12 + node_type_id: i3.xlarge + num_workers: 1 diff --git a/acceptance/bundle/variables/empty-in-task-params/out.deploy.direct.txt b/acceptance/bundle/variables/empty-in-task-params/out.deploy.direct.txt new file mode 100644 index 00000000000..25cd55d8a3f --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.deploy.direct.txt @@ -0,0 +1,6 @@ + +>>> errcode [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/empty-in-task-params/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! diff --git a/acceptance/bundle/variables/empty-in-task-params/out.deploy.terraform.txt b/acceptance/bundle/variables/empty-in-task-params/out.deploy.terraform.txt new file mode 100644 index 00000000000..3b24aaecc81 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.deploy.terraform.txt @@ -0,0 +1,23 @@ + +>>> errcode [CLI] bundle deploy +Warning: empty parameter in spark_python_task fails deploys that use the Terraform engine + at resources.jobs.demo_job.tasks[0].spark_python_task.parameters[1] + in databricks.yml:19:17 + +The Terraform provider reads an empty string element in a parameters list back as nil, and creating the job panics with "parameters[] is not a string". Remove the empty element or make its value non-empty. The direct engine (bundle.engine: direct) deploys empty parameters correctly. + +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/empty-in-task-params/default/files... +Deploying resources... +Error: terraform apply: exit status 1 + +Error: cannot create job: panic: task: spark_python_task: parameters: task.0.spark_python_task.0.parameters[] is not a string + + with databricks_job.demo_job, + on bundle.tf.json line 45, in resource.databricks_job.demo_job: + 45: } + + + +Updating deployment state... + +Exit code: 1 diff --git a/acceptance/bundle/variables/empty-in-task-params/out.jobs-create-params.direct.json b/acceptance/bundle/variables/empty-in-task-params/out.jobs-create-params.direct.json new file mode 100644 index 00000000000..2de25103123 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.jobs-create-params.direct.json @@ -0,0 +1,6 @@ +[ + [ + "--suffix", + "" + ] +] diff --git a/acceptance/bundle/variables/empty-in-task-params/out.jobs-create-params.terraform.json b/acceptance/bundle/variables/empty-in-task-params/out.jobs-create-params.terraform.json new file mode 100644 index 00000000000..fe51488c706 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.jobs-create-params.terraform.json @@ -0,0 +1 @@ +[] diff --git a/acceptance/bundle/variables/empty-in-task-params/out.test.toml b/acceptance/bundle/variables/empty-in-task-params/out.test.toml new file mode 100644 index 00000000000..67759662971 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false +GOOSOnPR.darwin = false +GOOSOnPR.windows = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/variables/empty-in-task-params/out.tf-params.terraform.json b/acceptance/bundle/variables/empty-in-task-params/out.tf-params.terraform.json new file mode 100644 index 00000000000..612ef5dd80b --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.tf-params.terraform.json @@ -0,0 +1,4 @@ +[ + "--suffix", + "" +] diff --git a/acceptance/bundle/variables/empty-in-task-params/out.validate.direct.txt b/acceptance/bundle/variables/empty-in-task-params/out.validate.direct.txt new file mode 100644 index 00000000000..f1d9808ad10 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.validate.direct.txt @@ -0,0 +1,9 @@ + +>>> errcode [CLI] bundle validate +Name: empty-in-task-params +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/empty-in-task-params/default + +Validation OK! diff --git a/acceptance/bundle/variables/empty-in-task-params/out.validate.terraform.txt b/acceptance/bundle/variables/empty-in-task-params/out.validate.terraform.txt new file mode 100644 index 00000000000..ab6fda0cb75 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/out.validate.terraform.txt @@ -0,0 +1,15 @@ + +>>> errcode [CLI] bundle validate +Warning: empty parameter in spark_python_task fails deploys that use the Terraform engine + at resources.jobs.demo_job.tasks[0].spark_python_task.parameters[1] + in databricks.yml:19:17 + +The Terraform provider reads an empty string element in a parameters list back as nil, and creating the job panics with "parameters[] is not a string". Remove the empty element or make its value non-empty. The direct engine (bundle.engine: direct) deploys empty parameters correctly. + +Name: empty-in-task-params +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/empty-in-task-params/default + +Found 1 warning diff --git a/acceptance/bundle/variables/empty-in-task-params/output.txt b/acceptance/bundle/variables/empty-in-task-params/output.txt new file mode 100644 index 00000000000..75dd999d6f1 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/output.txt @@ -0,0 +1,5 @@ + +=== Validate +=== Deploy +=== Parameters sent to jobs/create +>>> jq -s [.[] | select(.path | test("jobs/create")) | .body.tasks[0].spark_python_task.parameters] out.requests.txt diff --git a/acceptance/bundle/variables/empty-in-task-params/script b/acceptance/bundle/variables/empty-in-task-params/script new file mode 100644 index 00000000000..41a7fa629db --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/script @@ -0,0 +1,16 @@ +title "Validate" +trace errcode $CLI bundle validate > out.validate.$DATABRICKS_BUNDLE_ENGINE.txt 2>&1 + +title "Deploy" +trace errcode $CLI bundle deploy > out.deploy.$DATABRICKS_BUNDLE_ENGINE.txt 2>&1 + +title "Parameters sent to jobs/create" +trace jq -s '[.[] | select(.path | test("jobs/create")) | .body.tasks[0].spark_python_task.parameters]' out.requests.txt > out.jobs-create-params.$DATABRICKS_BUNDLE_ENGINE.json + +# The terraform engine also records the parameters as rendered into bundle.tf.json, +# to show the generated configuration is well formed (the empty string is ""). +if [ -f .databricks/bundle/default/terraform/bundle.tf.json ]; then + jq '.resource.databricks_job.demo_job.task[0].spark_python_task.parameters' .databricks/bundle/default/terraform/bundle.tf.json > out.tf-params.terraform.json +fi + +rm out.requests.txt diff --git a/acceptance/bundle/variables/empty-in-task-params/src/main.py b/acceptance/bundle/variables/empty-in-task-params/src/main.py new file mode 100644 index 00000000000..11b15b1a458 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/src/main.py @@ -0,0 +1 @@ +print("hello") diff --git a/acceptance/bundle/variables/empty-in-task-params/test.toml b/acceptance/bundle/variables/empty-in-task-params/test.toml new file mode 100644 index 00000000000..159efe02696 --- /dev/null +++ b/acceptance/bundle/variables/empty-in-task-params/test.toml @@ -0,0 +1 @@ +RecordRequests = true diff --git a/bundle/config/validate/empty_task_parameters.go b/bundle/config/validate/empty_task_parameters.go new file mode 100644 index 00000000000..391d8126ce0 --- /dev/null +++ b/bundle/config/validate/empty_task_parameters.go @@ -0,0 +1,107 @@ +package validate + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/engine" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/jobs" +) + +// EmptyTaskParameters warns when a task parameter is an empty string and the +// bundle deploys with the Terraform engine. The Terraform provider reads an +// empty list element back as nil and job creation panics with +// "parameters[] is not a string". The Jobs API accepts empty string +// parameters and the direct engine deploys them, so this is a warning and it +// is only emitted for the Terraform engine. +// +// Empty parameters usually come from a variable that is intentionally blank +// in some targets, e.g. a per-developer prefix passed as its own list element. +func EmptyTaskParameters() bundle.ReadOnlyMutator { + return &emptyTaskParameters{} +} + +type emptyTaskParameters struct{ bundle.RO } + +func (v *emptyTaskParameters) Name() string { + return "validate:empty_task_parameters" +} + +// Apply walks every job task (including tasks nested in a for_each_task) and +// collects a warning for each empty string element in a parameters list. It +// returns nothing when the effective engine is direct, because the direct +// engine deploys empty parameters correctly; the panic is specific to the +// Terraform provider. It must run after variable references in resources are +// resolved, so that a parameter written as ${var.x} is seen as its final +// string value. +func (v *emptyTaskParameters) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // Resolve effective engine: config takes precedence over env var. + effectiveEngine := b.Config.Bundle.Engine + if effectiveEngine == engine.EngineNotSet { + if envEngine, err := engine.FromEnv(ctx); err == nil { + effectiveEngine = envEngine + } + } + if effectiveEngine.IsDirect() { + return nil + } + + var diags diag.Diagnostics + for key, job := range b.Config.Resources.Jobs { + for i, task := range job.Tasks { + diags = diags.Extend(checkEmptyTaskParameters(b, &task, fmt.Sprintf("resources.jobs.%s.tasks[%d]", key, i))) + if task.ForEachTask != nil { + diags = diags.Extend(checkEmptyTaskParameters(b, &task.ForEachTask.Task, fmt.Sprintf("resources.jobs.%s.tasks[%d].for_each_task.task", key, i))) + } + } + } + return diags +} + +// checkEmptyTaskParameters returns one warning per empty string element in the +// given task's parameters list, covering the four task types whose parameters +// are a plain list of strings (spark_python_task, python_wheel_task, +// spark_jar_task, spark_submit_task). basePath is the config path of the task, +// e.g. "resources.jobs.my_job.tasks[0]"; it anchors each warning to the exact +// element ("..parameters[]") so the rendered diagnostic points +// at the offending line in the user's YAML. +func checkEmptyTaskParameters(b *bundle.Bundle, task *jobs.Task, basePath string) diag.Diagnostics { + var diags diag.Diagnostics + + check := func(params []string, taskType string) { + for i, p := range params { + if p != "" { + continue + } + path := fmt.Sprintf("%s.%s.parameters[%d]", basePath, taskType, i) + diags = append(diags, diag.Diagnostic{ + Severity: diag.Warning, + Summary: fmt.Sprintf("empty parameter in %s fails deploys that use the Terraform engine", taskType), + Detail: "The Terraform provider reads an empty string element in a parameters list back as nil, " + + "and creating the job panics with \"parameters[] is not a string\". " + + "Remove the empty element or make its value non-empty. " + + "The direct engine (bundle.engine: direct) deploys empty parameters correctly.", + Locations: b.Config.GetLocations(path), + Paths: []dyn.Path{dyn.MustPathFromString(path)}, + }) + } + } + + if task.SparkPythonTask != nil { + check(task.SparkPythonTask.Parameters, "spark_python_task") + } + if task.PythonWheelTask != nil { + check(task.PythonWheelTask.Parameters, "python_wheel_task") + } + if task.SparkJarTask != nil { + check(task.SparkJarTask.Parameters, "spark_jar_task") + } + if task.SparkSubmitTask != nil { + check(task.SparkSubmitTask.Parameters, "spark_submit_task") + } + + return diags +} diff --git a/bundle/config/validate/empty_task_parameters_test.go b/bundle/config/validate/empty_task_parameters_test.go new file mode 100644 index 00000000000..ed739b276da --- /dev/null +++ b/bundle/config/validate/empty_task_parameters_test.go @@ -0,0 +1,125 @@ +package validate + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/engine" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/require" +) + +func emptyTaskParametersBundle(eng engine.EngineType) *bundle.Bundle { + return &bundle.Bundle{ + Config: config.Root{ + Bundle: config.Bundle{ + Engine: eng, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + JobSettings: jobs.JobSettings{ + Name: "job1", + Tasks: []jobs.Task{ + { + TaskKey: "python", + SparkPythonTask: &jobs.SparkPythonTask{ + PythonFile: "./main.py", + Parameters: []string{"--suffix", ""}, + }, + }, + { + TaskKey: "wheel", + PythonWheelTask: &jobs.PythonWheelTask{ + PackageName: "pkg", + Parameters: []string{""}, + }, + }, + { + TaskKey: "ok", + SparkPythonTask: &jobs.SparkPythonTask{ + PythonFile: "./main.py", + Parameters: []string{"--suffix", "value"}, + }, + }, + { + TaskKey: "foreach", + ForEachTask: &jobs.ForEachTask{ + Task: jobs.Task{ + TaskKey: "inner", + SparkJarTask: &jobs.SparkJarTask{ + MainClassName: "Main", + Parameters: []string{""}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } +} + +func TestEmptyTaskParametersTerraformEngine(t *testing.T) { + b := emptyTaskParametersBundle(engine.EngineTerraform) + + diags := EmptyTaskParameters().Apply(t.Context(), b) + require.Len(t, diags, 3) + require.NoError(t, diags.Error()) + + var paths []string + for _, d := range diags { + require.Equal(t, diag.Warning, d.Severity) + require.Len(t, d.Paths, 1) + paths = append(paths, d.Paths[0].String()) + } + require.ElementsMatch(t, []string{ + "resources.jobs.job1.tasks[0].spark_python_task.parameters[1]", + "resources.jobs.job1.tasks[1].python_wheel_task.parameters[0]", + "resources.jobs.job1.tasks[3].for_each_task.task.spark_jar_task.parameters[0]", + }, paths) +} + +func TestEmptyTaskParametersDirectEngine(t *testing.T) { + b := emptyTaskParametersBundle(engine.EngineDirect) + + diags := EmptyTaskParameters().Apply(t.Context(), b) + require.Empty(t, diags) +} + +func TestEmptyTaskParametersNoEmptyValues(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Bundle: config.Bundle{ + Engine: engine.EngineTerraform, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + JobSettings: jobs.JobSettings{ + Name: "job1", + Tasks: []jobs.Task{ + { + TaskKey: "python", + SparkPythonTask: &jobs.SparkPythonTask{ + PythonFile: "./main.py", + Parameters: []string{"--suffix", "value"}, + }, + }, + }, + }, + }, + }, + }, + }, + } + + diags := EmptyTaskParameters().Apply(t.Context(), b) + require.Empty(t, diags) +} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index bfa2af4124b..d3626202fa9 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -182,6 +182,11 @@ func Initialize(ctx context.Context, b *bundle.Bundle) { // points to a Terraform-only field with no DABs equivalent. validate.TFOnlyReferences(), + // Reads (typed): resources.jobs.*.tasks (task parameters lists) + // Warns (TF engine only) when a task parameter is an empty string, because the + // Terraform provider reads it back as nil and job creation panics. + validate.EmptyTaskParameters(), + // Reads (typed): b.Config.Permissions (checks if current user or their groups have CAN_MANAGE permissions) // Reads (typed): b.Config.Workspace.CurrentUser (gets current user information) // Provides diagnostic recommendations if the current deployment identity isn't explicitly granted CAN_MANAGE permissions