diff --git a/.nextchanges/bundles/defaults-whole-reference.md b/.nextchanges/bundles/defaults-whole-reference.md new file mode 100644 index 00000000000..5c592840c65 --- /dev/null +++ b/.nextchanges/bundles/defaults-whole-reference.md @@ -0,0 +1 @@ +* Fixed `bundle validate` failing with "expected a map to index" when a job `schedule`, `trigger` or `continuous` is a reference to a whole block, such as `${resources.jobs.base.schedule}`. ([#7225](https://github.com/databricks/cli/pull/7225)) diff --git a/acceptance/bundle/validate/references/job_schedule_whole_reference/output.txt b/acceptance/bundle/validate/references/job_schedule_whole_reference/output.txt index 3709ef8711a..406a54ee03d 100644 --- a/acceptance/bundle/validate/references/job_schedule_whole_reference/output.txt +++ b/acceptance/bundle/validate/references/job_schedule_whole_reference/output.txt @@ -1,6 +1,12 @@ ->>> musterr [CLI] bundle validate -Error: expected a map to index "pause_status", found string +>>> [CLI] bundle validate +Warning: required field "quartz_cron_expression" is not set + at resources.jobs.viaref.schedule + in databricks.yml:9:17 + +Warning: required field "timezone_id" is not set + at resources.jobs.viaref.schedule + in databricks.yml:9:17 Name: test-bundle Target: default @@ -8,4 +14,19 @@ Workspace: User: [USERNAME] Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default -Found 1 error +Found 2 warnings + +>>> [CLI] bundle plan -o json +Warning: required field "quartz_cron_expression" is not set + at resources.jobs.viaref.schedule + in databricks.yml:9:17 + +Warning: required field "timezone_id" is not set + at resources.jobs.viaref.schedule + in databricks.yml:9:17 + +{ + "pause_status": "UNPAUSED", + "quartz_cron_expression": "0 0 * * * ?", + "timezone_id": "UTC" +} diff --git a/acceptance/bundle/validate/references/job_schedule_whole_reference/script b/acceptance/bundle/validate/references/job_schedule_whole_reference/script index 4c84b58bb8c..541f2548a6f 100644 --- a/acceptance/bundle/validate/references/job_schedule_whole_reference/script +++ b/acceptance/bundle/validate/references/job_schedule_whole_reference/script @@ -1,3 +1,4 @@ # The schedule of viaref is a reference to the whole schedule of another job. -# The defaults set schedule.pause_status (and trigger, continuous) of every job by path, which cannot be done in a reference. -trace musterr $CLI bundle validate +# The defaults (e.g. pause_status) are not written into the reference; the referenced schedule gets them. +trace $CLI bundle validate +trace $CLI bundle plan -o json | jq '.plan."resources.jobs.viaref".new_state.value.schedule' diff --git a/acceptance/bundle/validate/references/job_schedule_whole_reference/test.toml b/acceptance/bundle/validate/references/job_schedule_whole_reference/test.toml index 12ba67a70a9..6229ff00fb0 100644 --- a/acceptance/bundle/validate/references/job_schedule_whole_reference/test.toml +++ b/acceptance/bundle/validate/references/job_schedule_whole_reference/test.toml @@ -1 +1 @@ -Badness = "The default of schedule.pause_status is written by path, which fails with 'expected a map to index pause_status, found string' when the schedule is a reference to a whole schedule of another job; validate should accept it." +Badness = "The required-field check looks inside the reference: it warns that quartz_cron_expression and timezone_id are not set at resources.jobs.viaref.schedule, although the referenced schedule sets both." diff --git a/acceptance/bundle/validate/references/job_trigger_whole_reference/output.txt b/acceptance/bundle/validate/references/job_trigger_whole_reference/output.txt index 3709ef8711a..6563c4aa7a0 100644 --- a/acceptance/bundle/validate/references/job_trigger_whole_reference/output.txt +++ b/acceptance/bundle/validate/references/job_trigger_whole_reference/output.txt @@ -1,11 +1,17 @@ ->>> musterr [CLI] bundle validate -Error: expected a map to index "pause_status", found string - +>>> [CLI] bundle validate Name: test-bundle Target: default Workspace: User: [USERNAME] Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default -Found 1 error +Validation OK! + +>>> [CLI] bundle plan -o json +{ + "file_arrival": { + "url": "s3://bucket/path" + }, + "pause_status": "UNPAUSED" +} diff --git a/acceptance/bundle/validate/references/job_trigger_whole_reference/script b/acceptance/bundle/validate/references/job_trigger_whole_reference/script index b02a17e8dff..0c2071397f9 100644 --- a/acceptance/bundle/validate/references/job_trigger_whole_reference/script +++ b/acceptance/bundle/validate/references/job_trigger_whole_reference/script @@ -1,3 +1,4 @@ # The trigger of viaref is a reference to the whole trigger of another job. -# The defaults set trigger.pause_status (and trigger, continuous) of every job by path, which cannot be done in a reference. -trace musterr $CLI bundle validate +# The defaults (e.g. pause_status) are not written into the reference; the referenced trigger gets them. +trace $CLI bundle validate +trace $CLI bundle plan -o json | jq '.plan."resources.jobs.viaref".new_state.value.trigger' diff --git a/acceptance/bundle/validate/references/job_trigger_whole_reference/test.toml b/acceptance/bundle/validate/references/job_trigger_whole_reference/test.toml deleted file mode 100644 index 069b11f08be..00000000000 --- a/acceptance/bundle/validate/references/job_trigger_whole_reference/test.toml +++ /dev/null @@ -1 +0,0 @@ -Badness = "The default of trigger.pause_status is written by path, which fails with 'expected a map to index pause_status, found string' when the trigger is a reference to a whole trigger of another job; validate should accept it." diff --git a/bundle/set_default.go b/bundle/set_default.go index 3100f67d57c..aeb2a6f9843 100644 --- a/bundle/set_default.go +++ b/bundle/set_default.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/dynvar" "github.com/databricks/cli/libs/logdiag" ) @@ -30,6 +31,11 @@ func (m *setDefault) Name() string { func (m *setDefault) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { return dyn.MapByPattern(v, m.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + // A default cannot be written into a pure reference to a whole block + // (e.g. "${resources.jobs.base.schedule}"), so leave it to the referenced value. + if s, ok := v.AsString(); ok && dynvar.IsPureVariableReference(s) { + return v, nil + } _, err := dyn.GetByPath(v, m.key) switch { case dyn.IsNoSuchKeyError(err): diff --git a/bundle/set_default_test.go b/bundle/set_default_test.go new file mode 100644 index 00000000000..044906f14dd --- /dev/null +++ b/bundle/set_default_test.go @@ -0,0 +1,39 @@ +package bundle + +import ( + "testing" + + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/logdiag" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSetDefaultSkipsPureReference(t *testing.T) { + root, diags := config.LoadFromBytes("databricks.yml", []byte(` +resources: + jobs: + base: + schedule: + quartz_cron_expression: "0 0 * * * ?" + viaref: + schedule: ${resources.jobs.base.schedule} +`)) + require.Empty(t, diags) + b := &Bundle{Config: *root} + + ctx := logdiag.InitContext(t.Context()) + logdiag.SetCollect(ctx, true) + SetDefault(ctx, b, "resources.jobs.*.schedule.pause_status", "UNPAUSED") + require.Empty(t, logdiag.FlushCollected(ctx)) + + v := b.Config.Value() + base, err := dyn.GetByPath(v, dyn.MustPathFromString("resources.jobs.base.schedule.pause_status")) + require.NoError(t, err) + assert.Equal(t, "UNPAUSED", base.MustString()) + + viaref, err := dyn.GetByPath(v, dyn.MustPathFromString("resources.jobs.viaref.schedule")) + require.NoError(t, err) + assert.Equal(t, "${resources.jobs.base.schedule}", viaref.MustString()) +}