Skip to content

Commit d755193

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 d755193

14 files changed

Lines changed: 298 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: version_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles
8+
at resources.jobs.my_job.deployment.version_id
9+
in databricks.yml:11:21
10+
11+
Error: deployment_id must not be set in bundle configuration; it is managed by Databricks Asset Bundles
12+
at resources.pipelines.my_pipeline.deployment.deployment_id
13+
in databricks.yml:17:24
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: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package validate
2+
3+
import (
4+
"cmp"
5+
"context"
6+
"slices"
7+
8+
"github.com/databricks/cli/bundle"
9+
"github.com/databricks/cli/libs/diag"
10+
"github.com/databricks/cli/libs/dyn"
11+
)
12+
13+
func ValidateDeploymentFields() bundle.ReadOnlyMutator {
14+
return &validateDeploymentFields{}
15+
}
16+
17+
type validateDeploymentFields struct{ bundle.RO }
18+
19+
func (v *validateDeploymentFields) Name() string {
20+
return "validate:validate_deployment_fields"
21+
}
22+
23+
func (v *validateDeploymentFields) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
24+
var diags diag.Diagnostics
25+
26+
for name, job := range b.Config.Resources.Jobs {
27+
if job.Deployment == nil {
28+
continue
29+
}
30+
diags = diags.Extend(reservedDeploymentDiags(b, "resources.jobs."+name, job.Deployment.DeploymentId, job.Deployment.VersionId))
31+
}
32+
33+
for name, pipeline := range b.Config.Resources.Pipelines {
34+
if pipeline.Deployment == nil {
35+
continue
36+
}
37+
diags = diags.Extend(reservedDeploymentDiags(b, "resources.pipelines."+name, pipeline.Deployment.DeploymentId, pipeline.Deployment.VersionId))
38+
}
39+
40+
// Resources are stored in maps with randomized iteration order; sort by path
41+
// so the reported diagnostics are stable across runs.
42+
slices.SortFunc(diags, func(x, y diag.Diagnostic) int {
43+
return cmp.Compare(x.Paths[0].String(), y.Paths[0].String())
44+
})
45+
46+
return diags
47+
}
48+
49+
// reservedDeploymentDiags returns an error for each reserved deployment field
50+
// (deployment_id, version_id) set on the resource at resourcePath. These fields
51+
// are set by the CLI to identify the bundle deployment and its version in the
52+
// Deployment Metadata Service; a value set by hand would be overwritten at
53+
// deploy time, so we reject it up front.
54+
func reservedDeploymentDiags(b *bundle.Bundle, resourcePath, deploymentID, versionID string) diag.Diagnostics {
55+
var diags diag.Diagnostics
56+
for _, f := range []struct{ name, value string }{
57+
{"deployment_id", deploymentID},
58+
{"version_id", versionID},
59+
} {
60+
if f.value == "" {
61+
continue
62+
}
63+
path := resourcePath + ".deployment." + f.name
64+
diags = append(diags, diag.Diagnostic{
65+
Severity: diag.Error,
66+
Summary: f.name + " must not be set in bundle configuration; it is managed by Databricks Asset Bundles",
67+
Paths: []dyn.Path{dyn.MustPathFromString(path)},
68+
Locations: b.Config.GetLocations(path),
69+
})
70+
}
71+
return diags
72+
}
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/libs/diag"
10+
"github.com/databricks/databricks-sdk-go/service/jobs"
11+
"github.com/databricks/databricks-sdk-go/service/pipelines"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
const reservedFieldMsg = " must not be set in bundle configuration; it is managed by Databricks Asset Bundles"
17+
18+
func jobBundle(d *jobs.JobDeployment) *bundle.Bundle {
19+
return &bundle.Bundle{
20+
Config: config.Root{
21+
Resources: config.Resources{
22+
Jobs: map[string]*resources.Job{
23+
"my_job": {JobSettings: jobs.JobSettings{Name: "my_job", Deployment: d}},
24+
},
25+
},
26+
},
27+
}
28+
}
29+
30+
func pipelineBundle(d *pipelines.PipelineDeployment) *bundle.Bundle {
31+
return &bundle.Bundle{
32+
Config: config.Root{
33+
Resources: config.Resources{
34+
Pipelines: map[string]*resources.Pipeline{
35+
"my_pipeline": {CreatePipeline: pipelines.CreatePipeline{Name: "my_pipeline", Deployment: d}},
36+
},
37+
},
38+
},
39+
}
40+
}
41+
42+
func TestValidateDeploymentFieldsRejectsReservedFields(t *testing.T) {
43+
tests := []struct {
44+
name string
45+
b *bundle.Bundle
46+
want string
47+
}{
48+
{"job deployment_id", jobBundle(&jobs.JobDeployment{DeploymentId: "x"}), "deployment_id" + reservedFieldMsg},
49+
{"job version_id", jobBundle(&jobs.JobDeployment{VersionId: "x"}), "version_id" + reservedFieldMsg},
50+
{"pipeline deployment_id", pipelineBundle(&pipelines.PipelineDeployment{DeploymentId: "x"}), "deployment_id" + reservedFieldMsg},
51+
{"pipeline version_id", pipelineBundle(&pipelines.PipelineDeployment{VersionId: "x"}), "version_id" + reservedFieldMsg},
52+
}
53+
54+
for _, tt := range tests {
55+
t.Run(tt.name, func(t *testing.T) {
56+
diags := ValidateDeploymentFields().Apply(t.Context(), tt.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 := &bundle.Bundle{
66+
Config: config.Root{
67+
Resources: config.Resources{
68+
Jobs: map[string]*resources.Job{
69+
"my_job": {JobSettings: jobs.JobSettings{Deployment: &jobs.JobDeployment{DeploymentId: "a"}}},
70+
},
71+
Pipelines: map[string]*resources.Pipeline{
72+
"my_pipeline": {CreatePipeline: pipelines.CreatePipeline{Deployment: &pipelines.PipelineDeployment{VersionId: "b"}}},
73+
},
74+
},
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)