diff --git a/.nextchanges/bundles/7107.md b/.nextchanges/bundles/7107.md new file mode 100644 index 00000000000..913e44d6270 --- /dev/null +++ b/.nextchanges/bundles/7107.md @@ -0,0 +1 @@ +* Bundle validation now fails when a job table-update trigger lists more than one table without setting condition, instead of failing later at deploy. ([#7107](https://github.com/databricks/cli/pull/7107)) diff --git a/bundle/config/validate/fast_validate.go b/bundle/config/validate/fast_validate.go index d01eb8c1491..e7bbb397395 100644 --- a/bundle/config/validate/fast_validate.go +++ b/bundle/config/validate/fast_validate.go @@ -29,6 +29,7 @@ func (f *fastValidate) Apply(ctx context.Context, rb *bundle.Bundle) diag.Diagno // Fast mutators with only in-memory checks JobClusterKeyDefined(), JobTaskClusterSpec(), + JobTableUpdateCondition(), // Blocking mutators. Deployments will fail if these checks fail. ValidateArtifactPath(), diff --git a/bundle/config/validate/job_table_update_condition.go b/bundle/config/validate/job_table_update_condition.go new file mode 100644 index 00000000000..b230c928cf0 --- /dev/null +++ b/bundle/config/validate/job_table_update_condition.go @@ -0,0 +1,67 @@ +package validate + +import ( + "context" + "maps" + "slices" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" + "github.com/databricks/databricks-sdk-go/service/jobs" +) + +func JobTableUpdateCondition() bundle.ReadOnlyMutator { + return &jobTableUpdateCondition{} +} + +type jobTableUpdateCondition struct{ bundle.RO } + +func (v *jobTableUpdateCondition) Name() string { + return "validate:job_table_update_condition" +} + +func (v *jobTableUpdateCondition) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + // Map iteration order is randomized; sort by name for stable output. + for _, name := range slices.Sorted(maps.Keys(b.Config.Resources.Jobs)) { + job := b.Config.Resources.Jobs[name] + // An empty `jobs.:` entry loads as a present key with a nil value. + if job == nil { + continue + } + if job.Trigger != nil && job.Trigger.TableUpdate != nil { + path := structpath.NewPath(nil, "resources", "jobs", name, "trigger", "table_update") + diags = diags.Extend(checkTableUpdateCondition(b, job.Trigger.TableUpdate, path)) + } + // The Jobs API applies the same condition requirement to each entry + // of the plural `triggers` list. + for i := range job.Triggers { + if job.Triggers[i].TableUpdate == nil { + continue + } + path := structpath.NewPath(nil, "resources", "jobs", name, "triggers", i, "table_update") + diags = diags.Extend(checkTableUpdateCondition(b, job.Triggers[i].TableUpdate, path)) + } + } + + return diags +} + +func checkTableUpdateCondition(b *bundle.Bundle, tableUpdate *jobs.TableUpdateTriggerConfiguration, path *structpath.PathNode) diag.Diagnostics { + // The Jobs API rejects the trigger when more than one table is listed + // without a condition, so fail validation instead of failing at deploy. + if len(tableUpdate.TableNames) <= 1 || tableUpdate.Condition != "" { + return nil + } + // The condition field is missing from configuration, so point at the + // table_update block that holds a location. + return diag.Diagnostics{{ + Severity: diag.Error, + Summary: "trigger.table_update.condition is required when table_names lists more than one table", + Detail: "Set condition to ANY_UPDATED or ALL_UPDATED", + Paths: []*structpath.PathNode{path}, + Locations: b.Config.GetLocationsOf(path), + }} +} diff --git a/bundle/config/validate/job_table_update_condition_test.go b/bundle/config/validate/job_table_update_condition_test.go new file mode 100644 index 00000000000..d4fe70f2547 --- /dev/null +++ b/bundle/config/validate/job_table_update_condition_test.go @@ -0,0 +1,187 @@ +package validate + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "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 tableUpdateJob(condition jobs.Condition, tables ...string) *resources.Job { + return &resources.Job{ + Name: "job1", + Trigger: &jobs.TriggerSettings{ + TableUpdate: &jobs.TableUpdateTriggerConfiguration{ + Condition: condition, + TableNames: tables, + }, + }, + } +} + +func TestJobTableUpdateConditionMissing(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": tableUpdateJob("", "main.default.table_a", "main.default.table_b"), + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Len(t, diags, 1) + require.Equal(t, diag.Error, diags[0].Severity) + require.Error(t, diags.Error()) + require.Equal(t, "trigger.table_update.condition is required when table_names lists more than one table", diags[0].Summary) + require.Len(t, diags[0].Paths, 1) + require.Equal(t, "resources.jobs.job1.trigger.table_update", diags[0].Paths[0].String()) +} + +func TestJobTableUpdateConditionSet(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": tableUpdateJob(jobs.ConditionAnyUpdated, "main.default.table_a", "main.default.table_b"), + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Empty(t, diags) + require.NoError(t, diags.Error()) +} + +func TestJobTableUpdateConditionSingleTable(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": tableUpdateJob("", "main.default.table_a"), + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Empty(t, diags) + require.NoError(t, diags.Error()) +} + +func TestJobTableUpdateConditionAllUpdated(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": tableUpdateJob(jobs.ConditionAllUpdated, "main.default.table_a", "main.default.table_b"), + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Empty(t, diags) + require.NoError(t, diags.Error()) +} + +func tableUpdateTriggersJob(entries ...jobs.TriggerConfiguration) *resources.Job { + return &resources.Job{ + Name: "job1", + Triggers: entries, + } +} + +func tableUpdateTriggerEntry(condition jobs.Condition, tables ...string) jobs.TriggerConfiguration { + return jobs.TriggerConfiguration{ + TableUpdate: &jobs.TableUpdateTriggerConfiguration{ + Condition: condition, + TableNames: tables, + }, + } +} + +func TestJobTableUpdateConditionPluralTriggersMissing(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": tableUpdateTriggersJob( + tableUpdateTriggerEntry("", "main.default.table_a", "main.default.table_b"), + tableUpdateTriggerEntry(jobs.ConditionAnyUpdated, "main.default.table_a", "main.default.table_b"), + ), + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Len(t, diags, 1) + require.Equal(t, diag.Error, diags[0].Severity) + require.Equal(t, "trigger.table_update.condition is required when table_names lists more than one table", diags[0].Summary) + require.Len(t, diags[0].Paths, 1) + require.Equal(t, "resources.jobs.job1.triggers[0].table_update", diags[0].Paths[0].String()) +} + +func TestJobTableUpdateConditionPluralTriggersSingleTable(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": tableUpdateTriggersJob( + tableUpdateTriggerEntry("", "main.default.table_a"), + ), + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Empty(t, diags) + require.NoError(t, diags.Error()) +} + +func TestJobTableUpdateConditionOtherTriggerShapes(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + // A trigger without a table_update block is out of scope. + "job1": { + Name: "job1", + Trigger: &jobs.TriggerSettings{PauseStatus: jobs.PauseStatusUnpaused}, + }, + // Zero tables cannot violate the multi-table rule. + "job2": tableUpdateJob(""), + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Empty(t, diags) + require.NoError(t, diags.Error()) +} + +func TestJobTableUpdateConditionNoTrigger(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": {JobSettings: jobs.JobSettings{Name: "job1"}}, + "job2": nil, + }, + }, + }, + } + + diags := JobTableUpdateCondition().Apply(t.Context(), b) + require.Empty(t, diags) + require.NoError(t, diags.Error()) +}