Skip to content

Commit 5c00e17

Browse files
bundle: reject and hide deployment_id/version_id on jobs and pipelines
deployment_id and version_id on a job/pipeline deployment block are set by the CLI to track the bundle deployment in the Deployment Metadata Service. They are not user-configurable, so this change: - Errors at validation time if either is set in bundle configuration (new validate.ValidateDeploymentFields mutator). - Adds a hide_from_plan lifecycle annotation to the direct engine that removes a field from the plan diff (display only), and applies it to deployment.version_id for jobs and pipelines (version_id is set on every deploy, so showing it is pure noise). version_id also gets ignore_local_changes/ignore_remote_changes, which are what keep its churn from driving an update or showing as drift. deployment_id is intentionally left to show in the plan. - Removes deployment_id/version_id from the generated JSON schema. Co-authored-by: Isaac
1 parent e92f999 commit 5c00e17

14 files changed

Lines changed: 292 additions & 13 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
bundle:
2+
name: test-bundle
3+
4+
resources:
5+
jobs:
6+
my_job:
7+
name: my_job
8+
deployment:
9+
kind: BUNDLE
10+
deployment_id: "dep-123"
11+
version_id: "ver-456"
12+
pipelines:
13+
my_pipeline:
14+
name: my_pipeline
15+
deployment:
16+
kind: BUNDLE
17+
deployment_id: "dep-789"
18+
version_id: "ver-012"

acceptance/bundle/validate/reserved_deployment_fields/out.test.toml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
2+
>>> [CLI] bundle validate
3+
Error: deployment_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles
4+
at resources.jobs.my_job.deployment.deployment_id
5+
in databricks.yml:10:24
6+
7+
Error: deployment_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles
8+
at resources.pipelines.my_pipeline.deployment.deployment_id
9+
in databricks.yml:17:24
10+
11+
Error: version_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles
12+
at resources.jobs.my_job.deployment.version_id
13+
in databricks.yml:11:21
14+
15+
Error: version_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles
16+
at resources.pipelines.my_pipeline.deployment.version_id
17+
in databricks.yml:18:21
18+
19+
Name: test-bundle
20+
Target: default
21+
Workspace:
22+
User: [USERNAME]
23+
Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
24+
25+
Found 4 errors
26+
27+
Exit code: 1
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
trace $CLI bundle validate
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package validate
2+
3+
import (
4+
"cmp"
5+
"context"
6+
"fmt"
7+
"slices"
8+
9+
"github.com/databricks/cli/bundle"
10+
"github.com/databricks/cli/libs/diag"
11+
"github.com/databricks/cli/libs/dyn"
12+
)
13+
14+
// reservedDeploymentFields are fields on a resource's "deployment" block that
15+
// the CLI sets on the user's behalf: they identify the bundle deployment and
16+
// its version in the Deployment Metadata Service. A value set by hand would be
17+
// overwritten at deploy time, so we reject it up front with a clear error
18+
// rather than silently dropping it.
19+
var reservedDeploymentFields = []string{"deployment_id", "version_id"}
20+
21+
// reservedDeploymentResources are the resource types whose deployment block
22+
// carries the reserved fields above (jobs.JobDeployment, pipelines.PipelineDeployment).
23+
var reservedDeploymentResources = []string{"jobs", "pipelines"}
24+
25+
func ValidateDeploymentFields() bundle.ReadOnlyMutator {
26+
return &validateDeploymentFields{}
27+
}
28+
29+
type validateDeploymentFields struct{ bundle.RO }
30+
31+
func (v *validateDeploymentFields) Name() string {
32+
return "validate:validate_deployment_fields"
33+
}
34+
35+
func (v *validateDeploymentFields) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
36+
var diags diag.Diagnostics
37+
38+
for _, resourceType := range reservedDeploymentResources {
39+
for _, field := range reservedDeploymentFields {
40+
pattern := dyn.NewPattern(dyn.Key("resources"), dyn.Key(resourceType), dyn.AnyKey(), dyn.Key("deployment"), dyn.Key(field))
41+
_, err := dyn.MapByPattern(b.Config.Value(), pattern, func(p dyn.Path, val dyn.Value) (dyn.Value, error) {
42+
diags = append(diags, diag.Diagnostic{
43+
Severity: diag.Error,
44+
Summary: field + " must not be set in bundle configuration; it is managed by Databricks Asset Bundles",
45+
Paths: []dyn.Path{p},
46+
Locations: val.Locations(),
47+
})
48+
return val, nil
49+
})
50+
if err != nil {
51+
diags = diags.Extend(diag.FromErr(err))
52+
}
53+
}
54+
}
55+
56+
// MapByPattern visits resources in map order, which is randomized; sort so
57+
// the reported diagnostics are stable across runs.
58+
slices.SortFunc(diags, func(a, b diag.Diagnostic) int {
59+
if n := cmp.Compare(a.Summary, b.Summary); n != 0 {
60+
return n
61+
}
62+
return cmp.Compare(fmt.Sprintf("%v", a.Paths), fmt.Sprintf("%v", b.Paths))
63+
})
64+
65+
return diags
66+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package validate
2+
3+
import (
4+
"testing"
5+
6+
"github.com/databricks/cli/bundle"
7+
"github.com/databricks/cli/bundle/config"
8+
"github.com/databricks/cli/bundle/config/resources"
9+
"github.com/databricks/cli/bundle/internal/bundletest"
10+
"github.com/databricks/cli/libs/diag"
11+
"github.com/databricks/cli/libs/dyn"
12+
"github.com/databricks/databricks-sdk-go/service/jobs"
13+
"github.com/databricks/databricks-sdk-go/service/pipelines"
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/require"
16+
)
17+
18+
func deploymentFieldsBundle(t *testing.T) *bundle.Bundle {
19+
t.Helper()
20+
return &bundle.Bundle{
21+
Config: config.Root{
22+
Resources: config.Resources{
23+
Jobs: map[string]*resources.Job{
24+
"my_job": {JobSettings: jobs.JobSettings{Name: "my_job"}},
25+
},
26+
Pipelines: map[string]*resources.Pipeline{
27+
"my_pipeline": {CreatePipeline: pipelines.CreatePipeline{Name: "my_pipeline"}},
28+
},
29+
},
30+
},
31+
}
32+
}
33+
34+
func TestValidateDeploymentFieldsRejectsReservedFields(t *testing.T) {
35+
tests := []struct {
36+
name string
37+
resourcePath string
38+
field string
39+
want string
40+
}{
41+
{"job deployment_id", "resources.jobs.my_job.deployment", "deployment_id", "deployment_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles"},
42+
{"job version_id", "resources.jobs.my_job.deployment", "version_id", "version_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles"},
43+
{"pipeline deployment_id", "resources.pipelines.my_pipeline.deployment", "deployment_id", "deployment_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles"},
44+
{"pipeline version_id", "resources.pipelines.my_pipeline.deployment", "version_id", "version_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles"},
45+
}
46+
47+
for _, tt := range tests {
48+
t.Run(tt.name, func(t *testing.T) {
49+
b := deploymentFieldsBundle(t)
50+
bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) {
51+
return dyn.Set(v, tt.resourcePath, dyn.V(map[string]dyn.Value{
52+
tt.field: dyn.V("should-not-be-here"),
53+
}))
54+
})
55+
56+
diags := ValidateDeploymentFields().Apply(t.Context(), b)
57+
require.Len(t, diags, 1)
58+
assert.Equal(t, diag.Error, diags[0].Severity)
59+
assert.Equal(t, tt.want, diags[0].Summary)
60+
})
61+
}
62+
}
63+
64+
func TestValidateDeploymentFieldsReportsAllOffenders(t *testing.T) {
65+
b := deploymentFieldsBundle(t)
66+
bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) {
67+
v, err := dyn.Set(v, "resources.jobs.my_job.deployment", dyn.V(map[string]dyn.Value{
68+
"deployment_id": dyn.V("a"),
69+
}))
70+
if err != nil {
71+
return v, err
72+
}
73+
return dyn.Set(v, "resources.pipelines.my_pipeline.deployment", dyn.V(map[string]dyn.Value{
74+
"version_id": dyn.V("b"),
75+
}))
76+
})
77+
78+
diags := ValidateDeploymentFields().Apply(t.Context(), b)
79+
require.Len(t, diags, 2)
80+
}

bundle/direct/bundle_plan.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,12 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change
357357
return err
358358
}
359359

360-
if structdiff.IsEqual(ch.Remote, ch.New) {
360+
if shouldDropFromPlan(cfg, path) || shouldDropFromPlan(generatedCfg, path) {
361+
// Drop the field from the plan entirely. ReasonDrop removes the entry
362+
// below so it never surfaces as a change regardless of its diff state.
363+
ch.Action = deployplan.Skip
364+
ch.Reason = deployplan.ReasonDrop
365+
} else if structdiff.IsEqual(ch.Remote, ch.New) {
361366
ch.Action = deployplan.Skip
362367
ch.Reason = deployplan.ReasonRemoteAlreadySet
363368
} else if allEmpty(ch.Old, ch.New, ch.Remote) {
@@ -434,6 +439,16 @@ func shouldSkip(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNo
434439
return "", false
435440
}
436441

442+
// shouldDropFromPlan reports whether the field matches a HideFromPlan rule, meaning
443+
// it should be removed from the plan entirely rather than shown as a change.
444+
func shouldDropFromPlan(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNode) bool {
445+
if cfg == nil {
446+
return false
447+
}
448+
_, ok := findMatchingRule(path, cfg.HideFromPlan)
449+
return ok
450+
}
451+
437452
func shouldUpdateOrRecreate(cfg *dresources.ResourceLifecycleConfig, path *structpath.PathNode) (deployplan.ActionType, string) {
438453
if cfg == nil {
439454
return deployplan.Undefined, ""

bundle/direct/bundle_plan_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@ package direct
33
import (
44
"testing"
55

6+
"github.com/databricks/cli/bundle/direct/dresources"
67
"github.com/databricks/cli/libs/dyn"
8+
"github.com/databricks/cli/libs/structs/structpath"
79
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
811
)
912

1013
func TestDynPathToStructPath(t *testing.T) {
@@ -35,3 +38,19 @@ func TestDynPathToStructPath(t *testing.T) {
3538
assert.Equal(t, tc.expected, node.String())
3639
}
3740
}
41+
42+
// TestShouldDropFromPlanDeploymentVersion verifies that the DMS deployment
43+
// version_id is dropped from the plan for jobs and pipelines, while
44+
// deployment_id is left in (it is stable and fine to show).
45+
func TestShouldDropFromPlanDeploymentVersion(t *testing.T) {
46+
versionID, err := structpath.ParsePath("deployment.version_id")
47+
require.NoError(t, err)
48+
deploymentID, err := structpath.ParsePath("deployment.deployment_id")
49+
require.NoError(t, err)
50+
51+
for _, resourceType := range []string{"jobs", "pipelines"} {
52+
cfg := dresources.GetResourceConfig(resourceType)
53+
assert.True(t, shouldDropFromPlan(cfg, versionID), "%s: deployment.version_id should be dropped from plan", resourceType)
54+
assert.False(t, shouldDropFromPlan(cfg, deploymentID), "%s: deployment.deployment_id should not be dropped from plan", resourceType)
55+
}
56+
}

bundle/direct/dresources/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ type ResourceLifecycleConfig struct {
6262
// BackendDefaults: fields where the backend may set defaults.
6363
// When old and new are nil but remote is set, and the remote value matches allowed values (if specified), the change is skipped.
6464
BackendDefaults []BackendDefaultRule `yaml:"backend_defaults,omitempty"`
65+
66+
// HideFromPlan: field patterns removed from the plan diff entirely (never shown as a change).
67+
// This only controls display: it does not by itself decide whether a change is applied.
68+
// Use for fields the CLI sets on every deploy where the churn is pure noise (e.g.
69+
// deployment.version_id). Pair it with ignore_local_changes / ignore_remote_changes, which
70+
// are what actually keep the field from driving an update.
71+
HideFromPlan []FieldRule `yaml:"hide_from_plan,omitempty"`
6572
}
6673

6774
// Config is the root configuration structure for resource lifecycle behavior.
@@ -81,6 +88,7 @@ var empty = ResourceLifecycleConfig{
8188
RecreateOnChanges: nil,
8289
UpdateIDOnChanges: nil,
8390
BackendDefaults: nil,
91+
HideFromPlan: nil,
8492
}
8593

8694
func mustParseConfig(data []byte) func() *Config {

bundle/direct/dresources/config_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ func categoryRules(c ResourceLifecycleConfig) []struct {
3535
{"recreate_on_changes", c.RecreateOnChanges},
3636
{"update_id_on_changes", c.UpdateIDOnChanges},
3737
{"backend_defaults", backendAsFieldRules},
38+
{"hide_from_plan", c.HideFromPlan},
3839
}
3940
}
4041

0 commit comments

Comments
 (0)