Skip to content

Commit ab4a522

Browse files
denikIsaac
andcommitted
Keep main's path-node location lookups; build app permission paths from parts
Earlier rebases resolved conflicts with #6980 in favor of string paths, reverting call sites that main already builds with structpath.NewPath. Restore main's versions where this PR has no reason to change them, and make the app permission checks take a path node instead of parsing a string. Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 56ec7f2 commit ab4a522

12 files changed

Lines changed: 45 additions & 66 deletions

‎bundle/apps/validate.go‎

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/databricks/cli/bundle/config/resources"
1010
"github.com/databricks/cli/libs/diag"
1111
"github.com/databricks/cli/libs/structs/structpath"
12-
"github.com/databricks/cli/libs/structs/structvar"
1312
"github.com/databricks/databricks-sdk-go/service/apps"
1413
)
1514

@@ -28,7 +27,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
2827
Severity: diag.Error,
2928
Summary: "Missing app source code path or git source",
3029
Detail: fmt.Sprintf("app resource '%s' should have either source_code_path or git_source field", key),
31-
Locations: b.Config.GetLocations("resources.apps." + key),
30+
Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "apps", key)),
3231
})
3332
continue
3433
}
@@ -38,7 +37,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
3837
Severity: diag.Error,
3938
Summary: "Both source_code_path and git_source fields are set",
4039
Detail: fmt.Sprintf("app resource '%s' should have either source_code_path or git_source field, not both", key),
41-
Locations: b.Config.GetLocations("resources.apps." + key),
40+
Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "apps", key)),
4241
})
4342
continue
4443
}
@@ -48,7 +47,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
4847
Severity: diag.Error,
4948
Summary: "Duplicate app source code path",
5049
Detail: fmt.Sprintf("app resource '%s' has the same source code path as app resource '%s', this will lead to the app configuration being overridden by each other", key, usedSourceCodePaths[app.SourceCodePath]),
51-
Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)),
50+
Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "apps", key, "source_code_path")),
5251
})
5352
}
5453
usedSourceCodePaths[app.SourceCodePath] = key
@@ -92,12 +91,9 @@ type appResourceReference struct {
9291
permission string
9392
}
9493

95-
// hasPermissions checks if a bundle resource at the given dyn path has a non-empty permissions list.
96-
func hasPermissions(b *bundle.Bundle, resourcePath string) bool {
97-
pv, err := valueAt(b, resourcePath+".permissions")
98-
if err != nil {
99-
return false
100-
}
94+
// hasPermissions checks if a bundle resource at the given path has a non-empty permissions list.
95+
func hasPermissions(b *bundle.Bundle, resourcePath *structpath.PathNode) bool {
96+
pv := b.Config.View().Lookup(structpath.NewPath(resourcePath, "permissions"))
10197
for range pv.Sequence() {
10298
return true
10399
}
@@ -106,12 +102,9 @@ func hasPermissions(b *bundle.Bundle, resourcePath string) bool {
106102

107103
// hasAppSPInPermissions checks if any permission entry for the given resource
108104
// references the app's service principal via variable interpolation.
109-
func hasAppSPInPermissions(b *bundle.Bundle, resourcePath, appKey string) bool {
105+
func hasAppSPInPermissions(b *bundle.Bundle, resourcePath *structpath.PathNode, appKey string) bool {
110106
appSPRef := fmt.Sprintf("${resources.apps.%s.service_principal_client_id}", appKey)
111-
pv, err := valueAt(b, resourcePath+".permissions")
112-
if err != nil {
113-
return false
114-
}
107+
pv := b.Config.View().Lookup(structpath.NewPath(resourcePath, "permissions"))
115108
for _, entry := range pv.Sequence() {
116109
if str, ok := entry.Get("service_principal_name").AsString(); ok && str == appSPRef {
117110
return true
@@ -140,7 +133,7 @@ func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resourc
140133
}
141134
refType, resourceKey := matches[1], matches[2]
142135

143-
resourcePath := fmt.Sprintf("resources.%s.%s", refType, resourceKey)
136+
resourcePath := structpath.NewPath(nil, "resources", refType, resourceKey)
144137
if !hasPermissions(b, resourcePath) {
145138
continue
146139
}
@@ -149,7 +142,7 @@ func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resourc
149142
continue
150143
}
151144

152-
appPath := "resources.apps." + appKey
145+
appPath := structpath.NewPath(nil, "resources", "apps", appKey)
153146
diags = append(diags, diag.Diagnostic{
154147
Severity: diag.Warning,
155148
Summary: fmt.Sprintf("app %q references %s %q which has permissions set. To prevent permission override after deploying the app, please add the app service principal to the %s permissions", appKey, refType, resourceKey, refType),
@@ -167,8 +160,8 @@ func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resourc
167160
ref.permission,
168161
appKey,
169162
),
170-
Paths: structpath.NewPathSlice("resources", "apps", appKey),
171-
Locations: b.Config.GetLocations(appPath),
163+
Paths: []*structpath.PathNode{appPath},
164+
Locations: b.Config.GetLocationsOf(appPath),
172165
})
173166
}
174167

@@ -182,12 +175,3 @@ func (v *validate) Name() string {
182175
func Validate() bundle.Mutator {
183176
return &validate{}
184177
}
185-
186-
// valueAt returns the view of the configuration value at path.
187-
func valueAt(b *bundle.Bundle, path string) (structvar.View, error) {
188-
p, err := structpath.ParsePath(path)
189-
if err != nil {
190-
return structvar.View{}, err
191-
}
192-
return b.Config.View().Lookup(p), nil
193-
}

‎bundle/config/mutator/apply_source_linked_deployment_preset.go‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@ import (
1313

1414
type applySourceLinkedDeploymentPreset struct{}
1515

16-
func targetPresetPath(target string) *structpath.PathNode {
17-
return structpath.NewPath(nil, "targets", target, "presets", "source_linked_deployment")
18-
}
19-
2016
// Apply source-linked deployment preset
2117
func ApplySourceLinkedDeploymentPreset() *applySourceLinkedDeploymentPreset {
2218
return &applySourceLinkedDeploymentPreset{}
@@ -37,13 +33,13 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle
3733

3834
if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
3935
if !isDatabricksWorkspace {
40-
path := targetPresetPath(target)
36+
path := structpath.NewPath(nil, "targets", target, "presets", "source_linked_deployment")
4137
diags = diags.Append(
4238
diag.Diagnostic{
4339
Severity: diag.Warning,
4440
Summary: "source-linked deployment is available only in the Databricks Workspace",
4541
Paths: []*structpath.PathNode{path},
46-
Locations: b.Config.GetLocations("presets.source_linked_deployment"),
42+
Locations: b.Config.GetLocationsOf(path.SkipPrefix(2)),
4743
},
4844
)
4945

@@ -55,13 +51,13 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle
5551
b.Metrics.AddBoolValue("source_linked_set_for_non_development", b.Config.Bundle.Mode != config.Development)
5652

5753
if b.Config.Bundle.Mode != config.Development {
58-
path := targetPresetPath(target)
54+
path := structpath.NewPath(nil, "targets", target, "presets", "source_linked_deployment")
5955
diags = diags.Append(
6056
diag.Diagnostic{
6157
Severity: diag.Warning,
6258
Summary: "source-linked deployment in non-development mode is deprecated and will not be supported in a future release",
6359
Paths: []*structpath.PathNode{path},
64-
Locations: b.Config.GetLocations("presets.source_linked_deployment"),
60+
Locations: b.Config.GetLocationsOf(path.SkipPrefix(2)),
6561
},
6662
)
6763
}
@@ -74,14 +70,14 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle
7470

7571
// This mutator runs before workspace paths are defaulted so it's safe to check for the user-defined value
7672
if b.Config.Workspace.FilePath != "" && config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
77-
path := structpath.MustParsePath("workspace.file_path")
73+
path := structpath.NewPath(nil, "workspace", "file_path")
7874
diags = diags.Append(
7975
diag.Diagnostic{
8076
Severity: diag.Warning,
8177
Summary: "workspace.file_path setting will be ignored in source-linked deployment mode",
8278
Detail: "In source-linked deployment files are not copied to the destination and resources use source files instead",
8379
Paths: []*structpath.PathNode{path},
84-
Locations: b.Config.GetLocations(path.String()),
80+
Locations: b.Config.GetLocationsOf(path),
8581
},
8682
)
8783
}

‎bundle/config/mutator/resourcemutator/secret_scope_fixups.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,15 @@ func (m *secretScopeFixups) Apply(ctx context.Context, b *bundle.Bundle) diag.Di
131131
currentUser := b.Config.Workspace.CurrentUser.User
132132

133133
addManageForCurrentUser(scope, currentUser)
134-
path := structpath.NewPath(nil, "resources", "secret_scopes", key)
135134
err := collapsePermissions(scope)
136135
if err != nil {
137136
return diag.Diagnostics{
138137
{
139138
Severity: diag.Error,
140139
Summary: "Failed to collapse permissions for secret scope",
141140
Detail: err.Error(),
142-
Paths: []*structpath.PathNode{path},
143-
Locations: []diag.Location{b.Config.GetLocation(path.String())},
141+
Paths: structpath.NewPathSlice("resources", "secret_scopes", key),
142+
Locations: []diag.Location{b.Config.GetLocationOf(structpath.NewPath(nil, "resources", "secret_scopes", key))},
144143
},
145144
}
146145
}

‎bundle/config/mutator/sync_infer_root.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,12 @@ func (m *syncInferRoot) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno
9090
continue
9191
}
9292

93+
syncPath := structpath.NewPath(nil, "sync", "paths", i)
9394
diags = append(diags, diag.Diagnostic{
9495
Severity: diag.Error,
9596
Summary: fmt.Sprintf("invalid sync path %q", path),
96-
Locations: b.Config.GetLocations(fmt.Sprintf("sync.paths[%d]", i)),
97-
Paths: []*structpath.PathNode{structpath.NewIndex(structpath.MustParsePath("sync.paths"), i)},
97+
Locations: b.Config.GetLocationsOf(syncPath),
98+
Paths: []*structpath.PathNode{syncPath},
9899
})
99100
}
100101

‎bundle/config/validate/files_to_sync.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ func (v *filesToSync) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnost
5656
Summary: "There are no files to sync, please check your .gitignore",
5757
})
5858
} else {
59-
path := "sync.exclude"
59+
path := structpath.NewPath(nil, "sync", "exclude")
6060
diags = diags.Append(diag.Diagnostic{
6161
Severity: diag.Warning,
6262
Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration",
6363
// Show all locations where sync.exclude is defined, since merging
6464
// sync.exclude is additive.
65-
Locations: b.Config.GetLocations(path),
66-
Paths: structpath.MustParsePaths(path),
65+
Locations: b.Config.GetLocationsOf(path),
66+
Paths: []*structpath.PathNode{path},
6767
})
6868
}
6969

‎bundle/config/validate/job_cluster_key_defined.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,13 @@ func (v *jobClusterKeyDefined) Apply(ctx context.Context, b *bundle.Bundle) diag
3131
}
3232

3333
for index, task := range job.Tasks {
34-
taskPath := structpath.NewPath(nil, "resources", "jobs", k, "tasks", index)
3534
diags = diags.Extend(checkJobClusterKey(b, jobClusterKeys, task.JobClusterKey,
36-
structpath.NewPath(taskPath, "job_cluster_key")))
35+
structpath.NewPath(nil, "resources", "jobs", k, "tasks", index, "job_cluster_key")))
3736

3837
// The Jobs API rejects nested for_each_task, so one level is sufficient.
3938
if task.ForEachTask != nil {
4039
diags = diags.Extend(checkJobClusterKey(b, jobClusterKeys, task.ForEachTask.Task.JobClusterKey,
41-
structpath.NewPath(taskPath, "for_each_task", "task", "job_cluster_key")))
40+
structpath.NewPath(nil, "resources", "jobs", k, "tasks", index, "for_each_task", "task", "job_cluster_key")))
4241
}
4342
}
4443
}
@@ -61,7 +60,7 @@ func checkJobClusterKey(b *bundle.Bundle, jobClusterKeys map[string]bool, jobClu
6160
// Show only the location where the job_cluster_key is defined.
6261
// Other associated locations are not relevant since they are
6362
// overridden during merging.
64-
Locations: b.Config.GetLocations(path.String()),
63+
Locations: b.Config.GetLocationsOf(path),
6564
Paths: []*structpath.PathNode{path},
6665
}}
6766
}

‎bundle/config/validate/job_task_cluster_spec.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func validateJobTask(b *bundle.Bundle, task jobs.Task, taskPath *structpath.Path
9292
Severity: diag.Error,
9393
Summary: "Missing required cluster or environment settings",
9494
Detail: detail,
95-
Locations: b.Config.GetLocations(taskPath.String()),
95+
Locations: b.Config.GetLocationsOf(taskPath),
9696
Paths: []*structpath.PathNode{taskPath},
9797
})
9898
}

‎bundle/config/validate/required.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@ func errorForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnosti
9797
for key, dashboard := range b.Config.Resources.Dashboards {
9898
path := structpath.NewPath(nil, "resources", "dashboards", key)
9999
if dashboard.DisplayName == "" {
100-
nameLocations = append(nameLocations, b.Config.GetLocations(path.String())...)
100+
nameLocations = append(nameLocations, b.Config.GetLocationsOf(path)...)
101101
namePaths = append(namePaths, path)
102102
}
103103
if dashboard.WarehouseId == "" {
104-
warehouseIdLocations = append(warehouseIdLocations, b.Config.GetLocations(path.String())...)
104+
warehouseIdLocations = append(warehouseIdLocations, b.Config.GetLocationsOf(path)...)
105105
warehouseIdPaths = append(warehouseIdPaths, path)
106106
}
107107
}
@@ -131,7 +131,7 @@ func errorForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnosti
131131
diags = diags.Append(diag.Diagnostic{
132132
Severity: diag.Error,
133133
Summary: "sql_warehouse name is required",
134-
Locations: b.Config.GetLocations(path.String()),
134+
Locations: b.Config.GetLocationsOf(path),
135135
Paths: []*structpath.PathNode{path},
136136
})
137137
}
@@ -200,7 +200,7 @@ func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle
200200
Severity: diag.Error,
201201
Summary: "secret scope permission principal is required",
202202
Detail: "Set one of user_name, group_name or service_principal_name",
203-
Locations: b.Config.GetLocations(scopePath.String()),
203+
Locations: b.Config.GetLocationsOf(scopePath),
204204
Paths: []*structpath.PathNode{path},
205205
})
206206
}

‎bundle/config/validate/validate_artifact_path.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ func findVolumeInBundle(r config.Root, catalogName, schemaName, volumeName strin
6868
if v.SchemaName != schemaName && !isSchemaDefinedInBundle {
6969
continue
7070
}
71-
pathString := "resources.volumes." + k
72-
return structpath.MustParsePath(pathString), r.GetLocations(pathString), true
71+
volumePath := structpath.NewPath(nil, "resources", "volumes", k)
72+
return volumePath, r.GetLocationsOf(volumePath), true
7373
}
7474
return nil, nil, false
7575
}

‎bundle/config/validate/validate_deployment_fields.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,29 @@ func (v *validateDeploymentFields) Apply(_ context.Context, b *bundle.Bundle) di
2626
// deployment_id and version_id identify the bundle deployment and its version
2727
// in the Deployment Metadata Service. The CLI sets them on every deploy, so a
2828
// value provided by hand would be overwritten; reject it up front.
29-
reject := func(resource *structpath.PathNode, field, value string) {
29+
reject := func(resourceType, name, field, value string) {
3030
if value == "" {
3131
return
3232
}
33-
path := structpath.NewPath(resource, "deployment", field)
33+
path := structpath.NewPath(nil, "resources", resourceType, name, "deployment", field)
3434
diags = append(diags, diag.Diagnostic{
3535
Severity: diag.Error,
3636
Summary: field + " must not be set in bundle configuration; it is managed by Declarative Automation Bundles",
3737
Paths: []*structpath.PathNode{path},
38-
Locations: b.Config.GetLocations(path.String()),
38+
Locations: b.Config.GetLocationsOf(path),
3939
})
4040
}
4141

4242
for name, job := range b.Config.Resources.Jobs {
4343
if d := job.Deployment; d != nil {
44-
reject(structpath.NewPath(nil, "resources", "jobs", name), "deployment_id", d.DeploymentId)
45-
reject(structpath.NewPath(nil, "resources", "jobs", name), "version_id", d.VersionId)
44+
reject("jobs", name, "deployment_id", d.DeploymentId)
45+
reject("jobs", name, "version_id", d.VersionId)
4646
}
4747
}
4848
for name, pipeline := range b.Config.Resources.Pipelines {
4949
if d := pipeline.Deployment; d != nil {
50-
reject(structpath.NewPath(nil, "resources", "pipelines", name), "deployment_id", d.DeploymentId)
51-
reject(structpath.NewPath(nil, "resources", "pipelines", name), "version_id", d.VersionId)
50+
reject("pipelines", name, "deployment_id", d.DeploymentId)
51+
reject("pipelines", name, "version_id", d.VersionId)
5252
}
5353
}
5454

0 commit comments

Comments
 (0)