diff --git a/.nextchanges/bundles/name-prefix-reference.md b/.nextchanges/bundles/name-prefix-reference.md new file mode 100644 index 00000000000..00103620f1a --- /dev/null +++ b/.nextchanges/bundles/name-prefix-reference.md @@ -0,0 +1 @@ +* `presets.name_prefix` (and the development-mode prefix) is no longer added twice to a name that is a reference to the name of another resource, such as `name: ${resources.jobs.base.name}`. ([#7230](https://github.com/databricks/cli/pull/7230)) diff --git a/acceptance/bundle/presets/name_prefix_reference_to_name/output.txt b/acceptance/bundle/presets/name_prefix_reference_to_name/output.txt index 2a08a8c68d0..b6f4a5d44a4 100644 --- a/acceptance/bundle/presets/name_prefix_reference_to_name/output.txt +++ b/acceptance/bundle/presets/name_prefix_reference_to_name/output.txt @@ -1,3 +1,3 @@ >>> [CLI] bundle plan -o json -"pre-pre-base" +"pre-base" diff --git a/acceptance/bundle/presets/name_prefix_reference_to_name/script b/acceptance/bundle/presets/name_prefix_reference_to_name/script index 60df63696eb..5cc6675e053 100644 --- a/acceptance/bundle/presets/name_prefix_reference_to_name/script +++ b/acceptance/bundle/presets/name_prefix_reference_to_name/script @@ -1,2 +1,2 @@ -# name_prefix is added to a name that is a reference to the name of another job, which has the prefix too. +# The name of viaref is a reference to the name of another job, which has the prefix already. trace $CLI bundle plan -o json | jq '.plan["resources.jobs.viaref"].new_state.value.name' diff --git a/acceptance/bundle/presets/name_prefix_reference_to_name/test.toml b/acceptance/bundle/presets/name_prefix_reference_to_name/test.toml deleted file mode 100644 index 0f1ffaaa6c1..00000000000 --- a/acceptance/bundle/presets/name_prefix_reference_to_name/test.toml +++ /dev/null @@ -1 +0,0 @@ -Badness = "name_prefix is added a second time to a name that is a reference to the name of another job." diff --git a/acceptance/bundle/resource_deps/resources_var_presets/output.txt b/acceptance/bundle/resource_deps/resources_var_presets/output.txt index 7598641889d..013a004ce8f 100644 --- a/acceptance/bundle/resource_deps/resources_var_presets/output.txt +++ b/acceptance/bundle/resource_deps/resources_var_presets/output.txt @@ -23,7 +23,7 @@ }, "foo": { "catalog_name": "${resources.schemas.bar.catalog_name}", - "name": "dev_[USERNAME]_${resources.schemas.bar.name}" + "name": "${resources.schemas.bar.name}" } } } @@ -37,4 +37,4 @@ Files: 1 uploaded, 0 deleted Resources: 3 created, 0 changed, 0 deleted, 0 unchanged >>> jq -s .[] | select(.path=="/api/2.0/pipelines") | .body.name [OUTPUT_DIR]/out.requests.txt -"[dev [USERNAME]] pipeline for mycatalog.dev_[USERNAME]_dev_[USERNAME]_myschema" +"[dev [USERNAME]] pipeline for mycatalog.dev_[USERNAME]_myschema" diff --git a/bundle/config/mutator/resourcemutator/apply_presets.go b/bundle/config/mutator/resourcemutator/apply_presets.go index bb84240d7cc..97ddcf52ff4 100644 --- a/bundle/config/mutator/resourcemutator/apply_presets.go +++ b/bundle/config/mutator/resourcemutator/apply_presets.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/metrics" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn/dynvar" "github.com/databricks/cli/libs/textutil" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -59,7 +60,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if j == nil { continue } - j.Name = prefix + j.Name + j.Name = addPrefix(b, prefix, j.Name) if len(tags) > 0 { if j.Tags == nil { // Note: only create this map if tags is not empty, to avoid inserting "tags: {}" entry in the config @@ -97,7 +98,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if p == nil { continue } - p.Name = prefix + p.Name + p.Name = addPrefix(b, prefix, p.Name) if config.IsExplicitlyEnabled(t.PipelinesDevelopment) { //nolint:staticcheck // SA1019: pipeline development is deprecated in the SDK but remains a supported bundle config field p.Development = true @@ -125,7 +126,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if m == nil { continue } - m.Name = prefix + m.Name + m.Name = addPrefix(b, prefix, m.Name) for _, t := range tags { exists := slices.ContainsFunc(m.Tags, func(modelTag ml.ModelTag) bool { return modelTag.Key == t.Key @@ -142,13 +143,15 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if e == nil { continue } - filepath := e.Name - dir := path.Dir(filepath) - base := path.Base(filepath) - if dir == "." { - e.Name = prefix + base - } else { - e.Name = dir + "/" + prefix + base + if !refersToPrefixedName(b, e.Name) { + filepath := e.Name + dir := path.Dir(filepath) + base := path.Base(filepath) + if dir == "." { + e.Name = prefix + base + } else { + e.Name = dir + "/" + prefix + base + } } for _, t := range tags { exists := false @@ -169,7 +172,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if e == nil { continue } - e.Name = normalizePrefix(prefix) + e.Name + e.Name = addPrefix(b, normalizePrefix(prefix), e.Name) // As of 2024-06, model serving endpoints don't yet support tags } @@ -179,7 +182,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if m == nil { continue } - m.Name = normalizePrefix(prefix) + m.Name + m.Name = addPrefix(b, normalizePrefix(prefix), m.Name) // As of 2024-06, registered models don't yet support tags } @@ -208,7 +211,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if s == nil { continue } - s.Name = normalizePrefix(prefix) + s.Name + s.Name = addPrefix(b, normalizePrefix(prefix), s.Name) // HTTP API for schemas doesn't yet support tags. It's only supported in // the Databricks UI and via the SQL API. } @@ -218,7 +221,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if c == nil { continue } - c.ClusterName = prefix + c.ClusterName + c.ClusterName = addPrefix(b, prefix, c.ClusterName) if len(tags) > 0 { if c.CustomTags == nil { c.CustomTags = make(map[string]string, len(tags)) @@ -238,7 +241,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if pool == nil { continue } - pool.InstancePoolName = prefix + pool.InstancePoolName + pool.InstancePoolName = addPrefix(b, prefix, pool.InstancePoolName) if len(tags) > 0 { if pool.CustomTags == nil { pool.CustomTags = make(map[string]string, len(tags)) @@ -258,7 +261,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if dashboard == nil { continue } - dashboard.DisplayName = prefix + dashboard.DisplayName + dashboard.DisplayName = addPrefix(b, prefix, dashboard.DisplayName) } // Genie Spaces: Prefix @@ -266,7 +269,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if genieSpace == nil { continue } - genieSpace.Title = prefix + genieSpace.Title + genieSpace.Title = addPrefix(b, prefix, genieSpace.Title) } // Apps: No presets @@ -276,7 +279,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if a == nil { continue } - a.DisplayName = prefix + a.DisplayName + a.DisplayName = addPrefix(b, prefix, a.DisplayName) if t.TriggerPauseStatus != "" { paused := sql.SchedulePauseStatusPaused if t.TriggerPauseStatus == config.Unpaused { @@ -294,7 +297,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if w == nil { continue } - w.Name = prefix + w.Name + w.Name = addPrefix(b, prefix, w.Name) if len(tags) > 0 { if w.Tags == nil { w.Tags = &sql.EndpointTags{} @@ -329,7 +332,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos if cp == nil { continue } - cp.Name = prefix + cp.Name + cp.Name = addPrefix(b, prefix, cp.Name) } // Vector Search Endpoints: no prefix. The endpoint name is the primary key @@ -372,6 +375,48 @@ func toTagArray(tags map[string]string) []Tag { return tagArray } +// prefixedNameFields maps a resource type to the name field that ApplyPresets prefixes. +var prefixedNameFields = map[string]string{ + "jobs": "name", + "pipelines": "name", + "models": "name", + "experiments": "name", + "model_serving_endpoints": "name", + "registered_models": "name", + "schemas": "name", + "clusters": "cluster_name", + "instance_pools": "instance_pool_name", + "dashboards": "display_name", + "genie_spaces": "title", + "alerts": "display_name", + "sql_warehouses": "name", + "cluster_policies": "name", +} + +// addPrefix returns name with prefix, except when name is a reference to a name that +// is prefixed too. +func addPrefix(b *bundle.Bundle, prefix, name string) string { + if refersToPrefixedName(b, name) { + return name + } + return prefix + name +} + +// refersToPrefixedName reports whether name is a pure reference to a name that +// ApplyPresets prefixes too, e.g. "${resources.jobs.base.name}". The referenced +// value already has the prefix, so prefixing the reference would add it twice. +func refersToPrefixedName(b *bundle.Bundle, name string) bool { + p, ok := dynvar.PureReferenceToPath(name) + if !ok || len(p) != 4 || p[0].Key() != "resources" { + return false + } + if p[1].Key() == "schemas" && b.Config.Experimental != nil && b.Config.Experimental.SkipNamePrefixForSchema { + return false + } + field, ok := prefixedNameFields[p[1].Key()] + return ok && p[3].Key() == field +} + // normalizePrefix prefixes strings like '[dev lennart] ' to 'dev_lennart_'. // We leave unicode letters and numbers but remove all "special characters." func normalizePrefix(prefix string) string { diff --git a/bundle/config/mutator/resourcemutator/apply_presets_test.go b/bundle/config/mutator/resourcemutator/apply_presets_test.go index c97db137b26..2caa2812d1a 100644 --- a/bundle/config/mutator/resourcemutator/apply_presets_test.go +++ b/bundle/config/mutator/resourcemutator/apply_presets_test.go @@ -7,6 +7,9 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/databricks-sdk-go/service/catalog" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/databricks/databricks-sdk-go/service/ml" + "github.com/databricks/databricks-sdk-go/service/pipelines" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -38,3 +41,48 @@ func TestApplyPresetsSkipsNilQualityMonitors(t *testing.T) { require.NoError(t, diags.Error()) assert.Nil(t, b.Config.Resources.QualityMonitors["monitor"].Schedule) } + +func TestApplyPresetsNamePrefixReferenceToName(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Presets: config.Presets{ + NamePrefix: "pre-", + }, + Experimental: &config.Experimental{ + SkipNamePrefixForSchema: true, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "base": {JobSettings: jobs.JobSettings{Name: "base"}}, + "to_name": {JobSettings: jobs.JobSettings{Name: "${resources.jobs.base.name}"}}, + "to_id": {JobSettings: jobs.JobSettings{Name: "${resources.jobs.base.id}"}}, + "to_schema": {JobSettings: jobs.JobSettings{Name: "${resources.schemas.s.name}"}}, + "not_pure": {JobSettings: jobs.JobSettings{Name: "${resources.jobs.base.name} copy"}}, + }, + Pipelines: map[string]*resources.Pipeline{ + "to_job": {CreatePipeline: pipelines.CreatePipeline{Name: "${resources.jobs.base.name}"}}, + }, + Experiments: map[string]*resources.MlflowExperiment{ + "to_experiment": {CreateExperiment: ml.CreateExperiment{Name: "${resources.experiments.e.name}"}}, + }, + RegisteredModels: map[string]*resources.RegisteredModel{ + "to_model": {CreateRegisteredModelRequest: catalog.CreateRegisteredModelRequest{Name: "${resources.registered_models.m.name}"}}, + }, + }, + }, + } + + diags := bundle.Apply(t.Context(), b, ApplyPresets()) + require.NoError(t, diags.Error()) + + r := b.Config.Resources + assert.Equal(t, "pre-base", r.Jobs["base"].Name) + assert.Equal(t, "${resources.jobs.base.name}", r.Jobs["to_name"].Name) + assert.Equal(t, "pre-${resources.jobs.base.id}", r.Jobs["to_id"].Name) + // Schemas are not prefixed with skip_name_prefix_for_schema, so the reference is. + assert.Equal(t, "pre-${resources.schemas.s.name}", r.Jobs["to_schema"].Name) + assert.Equal(t, "pre-${resources.jobs.base.name} copy", r.Jobs["not_pure"].Name) + assert.Equal(t, "${resources.jobs.base.name}", r.Pipelines["to_job"].Name) + assert.Equal(t, "${resources.experiments.e.name}", r.Experiments["to_experiment"].Name) + assert.Equal(t, "${resources.registered_models.m.name}", r.RegisteredModels["to_model"].Name) +}