From fbe75755870181cc26570c4f026fc6b48ec0d435 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 7 Oct 2026 23:24:06 +0200 Subject: [PATCH 1/2] structpath: build computed paths and patterns from parts; lint rule Co-authored-by: Isaac --- .agents/rules/style-guide-go.md | 4 +++ .../mutator/paths/artifact_paths_visitor.go | 4 +-- .../paths/job_libraries_paths_visitor.go | 10 +++--- .../config/mutator/paths/job_paths_visitor.go | 23 ++++++++------ .../mutator/paths/pipeline_paths_visitor.go | 10 +++--- .../mutator/resolve_variable_references.go | 3 +- .../apply_bundle_permissions.go | 2 +- bundle/config/mutator/rewrite_sync_paths.go | 2 +- bundle/config/validate/files_to_sync.go | 5 ++- .../config/validate/validate_artifact_path.go | 4 +-- .../config/validate/validate_volume_path.go | 13 ++++---- bundle/libraries/remote_path.go | 16 +++++----- bundle/libraries/same_name_libraries.go | 16 +++++----- libs/gorules/rule_structpath_parse.go | 26 ++++++++++++++++ libs/structs/structpath/path.go | 31 +++++++++++++++++++ libs/structs/structpath/path_test.go | 10 ++++++ 16 files changed, 126 insertions(+), 53 deletions(-) create mode 100644 libs/gorules/rule_structpath_parse.go diff --git a/.agents/rules/style-guide-go.md b/.agents/rules/style-guide-go.md index bedd86132ca..07847641fdc 100644 --- a/.agents/rules/style-guide-go.md +++ b/.agents/rules/style-guide-go.md @@ -117,6 +117,10 @@ return fieldPaths **RULE: Be careful with `encoding/csv` `Writer.UseCRLF = true`.** It rewrites both record terminators AND embedded newlines inside quoted fields to `\r\n`, so tests for quoted multiline fields must expect `\r\n`, not just the line endings between rows. +### Structpath + +**RULE: Build computed structpath paths and patterns from parts (`structpath.NewPath`, `NewPathSlice`, `NewPattern`); never format or concatenate a string to parse it.** `MustParsePath`/`MustParsePattern`/`MustParsePaths` are for string literals and tests; `ParsePath` with the error handled is for user input. The `NoComputedStructpathParse` ruleguard rule enforces this. + ### Environment variables **RULE: In library and product code, use `github.com/databricks/cli/libs/env` for reading environment variables, not `os.Getenv`.** `env.Get(ctx, name)` and `env.Lookup(ctx, name)` can be overridden per-context in tests, so you don't have to mutate process-wide state to exercise a code path. `os.Getenv` is still fine in `main`, tests, and acceptance/integration harnesses where no `ctx` is available and overrides aren't needed. diff --git a/bundle/config/mutator/paths/artifact_paths_visitor.go b/bundle/config/mutator/paths/artifact_paths_visitor.go index 4f934e42ba8..017d8d1835a 100644 --- a/bundle/config/mutator/paths/artifact_paths_visitor.go +++ b/bundle/config/mutator/paths/artifact_paths_visitor.go @@ -12,12 +12,12 @@ type artifactRewritePattern struct { func artifactRewritePatterns() []artifactRewritePattern { // Base pattern to match all artifacts. - base := "artifacts.*" + base := structpath.NewPattern(nil, "artifacts", structpath.AnyKey) // Compile list of configuration paths to rewrite. return []artifactRewritePattern{ { - pattern: structpath.MustParsePattern(base + ".path"), + pattern: structpath.NewPattern(base, "path"), mode: TranslateModeLocalAbsoluteDirectory, }, } diff --git a/bundle/config/mutator/paths/job_libraries_paths_visitor.go b/bundle/config/mutator/paths/job_libraries_paths_visitor.go index e96e136f814..0e20ac565c0 100644 --- a/bundle/config/mutator/paths/job_libraries_paths_visitor.go +++ b/bundle/config/mutator/paths/job_libraries_paths_visitor.go @@ -6,15 +6,15 @@ import ( "github.com/databricks/cli/libs/structs/structvar" ) -func jobTaskLibrariesRewritePatterns(base string) []jobRewritePattern { +func jobTaskLibrariesRewritePatterns(base *structpath.PatternNode) []jobRewritePattern { return []jobRewritePattern{ { - structpath.MustParsePattern(base + ".libraries[*].whl"), + structpath.NewPattern(base, "libraries", structpath.AnyIndex, "whl"), TranslateModeLocalRelative, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".libraries[*].jar"), + structpath.NewPattern(base, "libraries", structpath.AnyIndex, "jar"), TranslateModeLocalRelative, noSkipRewrite, }, @@ -23,7 +23,7 @@ func jobTaskLibrariesRewritePatterns(base string) []jobRewritePattern { func jobLibrariesRewritePatterns() []jobRewritePattern { // Base pattern to match all tasks in all jobs. - base := "resources.jobs.*.tasks[*]" + base := jobTasksPattern // Compile list of patterns and their respective rewrite functions. jobEnvironmentsPatterns := []jobRewritePattern{ @@ -48,7 +48,7 @@ func jobLibrariesRewritePatterns() []jobRewritePattern { } taskPatterns := jobTaskLibrariesRewritePatterns(base) - forEachPatterns := jobTaskLibrariesRewritePatterns(base + ".for_each_task.task") + forEachPatterns := jobTaskLibrariesRewritePatterns(structpath.NewPattern(base, "for_each_task", "task")) allPatterns := append(taskPatterns, jobEnvironmentsPatterns...) allPatterns = append(allPatterns, jobEnvironmentsWithRequirementsPatterns...) allPatterns = append(allPatterns, forEachPatterns...) diff --git a/bundle/config/mutator/paths/job_paths_visitor.go b/bundle/config/mutator/paths/job_paths_visitor.go index 9928083c586..3bcf5edf0cc 100644 --- a/bundle/config/mutator/paths/job_paths_visitor.go +++ b/bundle/config/mutator/paths/job_paths_visitor.go @@ -11,39 +11,42 @@ type jobRewritePattern struct { skipRewrite func(string) bool } +// jobTasksPattern matches all tasks in all jobs. +var jobTasksPattern = structpath.NewPattern(nil, "resources", "jobs", structpath.AnyKey, "tasks", structpath.AnyIndex) + func noSkipRewrite(string) bool { return false } -func jobTaskRewritePatterns(base string) []jobRewritePattern { +func jobTaskRewritePatterns(base *structpath.PatternNode) []jobRewritePattern { return []jobRewritePattern{ { - structpath.MustParsePattern(base + ".notebook_task.notebook_path"), + structpath.NewPattern(base, "notebook_task", "notebook_path"), TranslateModeNotebook, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".spark_python_task.python_file"), + structpath.NewPattern(base, "spark_python_task", "python_file"), TranslateModeFile, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".dbt_task.project_directory"), + structpath.NewPattern(base, "dbt_task", "project_directory"), TranslateModeDirectory, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".sql_task.file.path"), + structpath.NewPattern(base, "sql_task", "file", "path"), TranslateModeFile, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".alert_task.workspace_path"), + structpath.NewPattern(base, "alert_task", "workspace_path"), TranslateModeFile, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".libraries[*].requirements"), + structpath.NewPattern(base, "libraries", structpath.AnyIndex, "requirements"), TranslateModeFile, noSkipRewrite, }, @@ -51,7 +54,7 @@ func jobTaskRewritePatterns(base string) []jobRewritePattern { // The AI Runtime task runs this bash script on each node; the backend // reads it as a workspace file, so translate the local path to its // remote (or immutable-snapshot) location like any other file. - structpath.MustParsePattern(base + ".ai_runtime_task.deployments[*].command_path"), + structpath.NewPattern(base, "ai_runtime_task", "deployments", structpath.AnyIndex, "command_path"), TranslateModeFile, noSkipRewrite, }, @@ -60,10 +63,10 @@ func jobTaskRewritePatterns(base string) []jobRewritePattern { func jobRewritePatterns() []jobRewritePattern { // Base pattern to match all tasks in all jobs. - base := "resources.jobs.*.tasks[*]" + base := jobTasksPattern taskPatterns := jobTaskRewritePatterns(base) - forEachPatterns := jobTaskRewritePatterns(base + ".for_each_task.task") + forEachPatterns := jobTaskRewritePatterns(structpath.NewPattern(base, "for_each_task", "task")) patterns := append(taskPatterns, forEachPatterns...) return append(patterns, jobRewritePattern{ diff --git a/bundle/config/mutator/paths/pipeline_paths_visitor.go b/bundle/config/mutator/paths/pipeline_paths_visitor.go index ff50c637db8..23b7c171462 100644 --- a/bundle/config/mutator/paths/pipeline_paths_visitor.go +++ b/bundle/config/mutator/paths/pipeline_paths_visitor.go @@ -16,28 +16,28 @@ type pipelineRewritePattern struct { } // Base pattern to match all libraries in all pipelines. -var base = "resources.pipelines.*" +var base = structpath.NewPattern(nil, "resources", "pipelines", structpath.AnyKey) func pipelineRewritePatterns() []pipelineRewritePattern { // Compile list of configuration paths to rewrite. allPatterns := []pipelineRewritePattern{ { - pattern: structpath.MustParsePattern(base + ".libraries[*].notebook.path"), + pattern: structpath.NewPattern(base, "libraries", structpath.AnyIndex, "notebook", "path"), mode: TranslateModeNotebook, skipRewrite: noSkipRewrite, }, { - pattern: structpath.MustParsePattern(base + ".libraries[*].file.path"), + pattern: structpath.NewPattern(base, "libraries", structpath.AnyIndex, "file", "path"), mode: TranslateModeFile, skipRewrite: noSkipRewrite, }, { - pattern: structpath.MustParsePattern(base + ".libraries[*].glob.include"), + pattern: structpath.NewPattern(base, "libraries", structpath.AnyIndex, "glob", "include"), mode: TranslateModeGlob, skipRewrite: noSkipRewrite, }, { - pattern: structpath.MustParsePattern(base + ".root_path"), + pattern: structpath.NewPattern(base, "root_path"), mode: TranslateModeDirectory, skipRewrite: noSkipRewrite, }, diff --git a/bundle/config/mutator/resolve_variable_references.go b/bundle/config/mutator/resolve_variable_references.go index f1e779ad832..85babc91d5a 100644 --- a/bundle/config/mutator/resolve_variable_references.go +++ b/bundle/config/mutator/resolve_variable_references.go @@ -47,6 +47,7 @@ var defaultPrefixes = []string{ var artifactPath = structpath.MustParsePath("artifacts") type resolveVariableReferences struct { + // prefixes are top-level config keys. prefixes []string pattern *structpath.PatternNode lookupFn func(structvar.View, *structpath.PathNode, *bundle.Bundle) (structvar.View, error) @@ -169,7 +170,7 @@ func (m *resolveVariableReferences) Validate(ctx context.Context, b *bundle.Bund func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { prefixes := make([]*structpath.PathNode, len(m.prefixes)) for i, prefix := range m.prefixes { - prefixes[i] = structpath.MustParsePath(prefix) + prefixes[i] = structpath.NewPath(nil, prefix) } // The path ${var.foo} is a shorthand for ${variables.foo.value}. diff --git a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go index bfc9e373616..96a819c57f1 100644 --- a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go +++ b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go @@ -116,7 +116,7 @@ func (m *bundlePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Di slices.Sort(keys) for _, key := range keys { - pattern := structpath.MustParsePattern("resources." + key + ".*") + pattern := structpath.NewPattern(nil, "resources", key, structpath.AnyKey) err = structvar.ForEach(b.Config.View(), pattern, func(p *structpath.PathNode, v structvar.View) error { had := v.Get("permissions").IsValid() diff --git a/bundle/config/mutator/rewrite_sync_paths.go b/bundle/config/mutator/rewrite_sync_paths.go index c4eefebae2e..91a3451e4b6 100644 --- a/bundle/config/mutator/rewrite_sync_paths.go +++ b/bundle/config/mutator/rewrite_sync_paths.go @@ -48,7 +48,7 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string, v structvar.View) (string func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { rewrite := func(field string, toSlash bool) error { - pattern := structpath.MustParsePattern("sync." + field + "[*]") + pattern := structpath.NewPattern(nil, "sync", field, structpath.AnyIndex) return structvar.ForEach(b.Config.View(), pattern, func(p *structpath.PathNode, v structvar.View) error { path, err := m.makeRelativeTo(b.BundleRootPath, v) if err != nil { diff --git a/bundle/config/validate/files_to_sync.go b/bundle/config/validate/files_to_sync.go index d1b811e17a7..b2408bbf3ce 100644 --- a/bundle/config/validate/files_to_sync.go +++ b/bundle/config/validate/files_to_sync.go @@ -56,14 +56,13 @@ func (v *filesToSync) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnost Summary: "There are no files to sync, please check your .gitignore", }) } else { - path := "sync.exclude" diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration", // Show all locations where sync.exclude is defined, since merging // sync.exclude is additive. - Locations: b.Config.GetLocations(path), - Paths: structpath.MustParsePaths(path), + Locations: b.Config.GetLocations("sync.exclude"), + Paths: structpath.NewPathSlice("sync", "exclude"), }) } diff --git a/bundle/config/validate/validate_artifact_path.go b/bundle/config/validate/validate_artifact_path.go index 1eaa41518cc..0989523d4a9 100644 --- a/bundle/config/validate/validate_artifact_path.go +++ b/bundle/config/validate/validate_artifact_path.go @@ -68,8 +68,8 @@ func findVolumeInBundle(r config.Root, catalogName, schemaName, volumeName strin if v.SchemaName != schemaName && !isSchemaDefinedInBundle { continue } - pathString := "resources.volumes." + k - return structpath.MustParsePath(pathString), r.GetLocations(pathString), true + volumePath := structpath.NewPath(nil, "resources", "volumes", k) + return volumePath, r.GetLocations(volumePath.String()), true } return nil, nil, false } diff --git a/bundle/config/validate/validate_volume_path.go b/bundle/config/validate/validate_volume_path.go index c440dfbf06f..802034bbbee 100644 --- a/bundle/config/validate/validate_volume_path.go +++ b/bundle/config/validate/validate_volume_path.go @@ -23,13 +23,12 @@ func (m *validateVolumePath) Apply(ctx context.Context, b *bundle.Bundle) diag.D // Define paths to check and their corresponding config field names pathChecks := []struct { path string - configName string configPath *structpath.PathNode }{ - {b.Config.Workspace.RootPath, "workspace.root_path", structpath.NewPath(nil, "workspace", "root_path")}, - {b.Config.Workspace.FilePath, "workspace.file_path", structpath.NewPath(nil, "workspace", "file_path")}, - {b.Config.Workspace.StatePath, "workspace.state_path", structpath.NewPath(nil, "workspace", "state_path")}, - {b.Config.Workspace.ResourcePath, "workspace.resource_path", structpath.NewPath(nil, "workspace", "resource_path")}, + {b.Config.Workspace.RootPath, structpath.NewPath(nil, "workspace", "root_path")}, + {b.Config.Workspace.FilePath, structpath.NewPath(nil, "workspace", "file_path")}, + {b.Config.Workspace.StatePath, structpath.NewPath(nil, "workspace", "state_path")}, + {b.Config.Workspace.ResourcePath, structpath.NewPath(nil, "workspace", "resource_path")}, } // Check each path @@ -37,14 +36,14 @@ func (m *validateVolumePath) Apply(ctx context.Context, b *bundle.Bundle) diag.D if check.path != "" && strings.HasPrefix(check.path, "/Volumes/") { diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, - Summary: fmt.Sprintf("%s %s starts with /Volumes. /Volumes can only be used with workspace.artifact_path.", check.configName, check.path), + Summary: fmt.Sprintf("%s %s starts with /Volumes. /Volumes can only be used with workspace.artifact_path.", check.configPath, check.path), Detail: "For more information, see https://docs.databricks.com/aws/en/dev-tools/bundles/settings#workspace", Locations: b.Config.GetLocationsOf(check.configPath), Paths: []*structpath.PathNode{check.configPath}, }) // Return early for root path validation - if check.configName == "workspace.root_path" { + if check.configPath.String() == "workspace.root_path" { return diags } } diff --git a/bundle/libraries/remote_path.go b/bundle/libraries/remote_path.go index d28e21c10c1..9c890d98d4a 100644 --- a/bundle/libraries/remote_path.go +++ b/bundle/libraries/remote_path.go @@ -131,14 +131,14 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]LocationToUpdate, err libs := make(map[string]([]LocationToUpdate)) patterns := []*structpath.PatternNode{ - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(envDepsPattern.String() + "[*]"), - structpath.MustParsePattern(pipelineEnvDepsPattern.String() + "[*]"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(envDepsPattern, structpath.AnyIndex), + structpath.NewPattern(pipelineEnvDepsPattern, structpath.AnyIndex), // The AI Runtime task's code_source_path is a local archive (typically an // artifact-built .tar.gz) that must be uploaded and referenced by its remote // path, exactly like a wheel or jar library. diff --git a/bundle/libraries/same_name_libraries.go b/bundle/libraries/same_name_libraries.go index a2ba9591a4f..3dcb4ba03b0 100644 --- a/bundle/libraries/same_name_libraries.go +++ b/bundle/libraries/same_name_libraries.go @@ -14,14 +14,14 @@ import ( type checkForSameNameLibraries struct{} var patterns = []*structpath.PatternNode{ - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(envDepsPattern.String() + "[*]"), - structpath.MustParsePattern(pipelineEnvDepsPattern.String() + "[*]"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(envDepsPattern, structpath.AnyIndex), + structpath.NewPattern(pipelineEnvDepsPattern, structpath.AnyIndex), } type libData struct { diff --git a/libs/gorules/rule_structpath_parse.go b/libs/gorules/rule_structpath_parse.go new file mode 100644 index 00000000000..672aabd5a73 --- /dev/null +++ b/libs/gorules/rule_structpath_parse.go @@ -0,0 +1,26 @@ +package gorules + +import "github.com/quasilyte/go-ruleguard/dsl" + +// NoComputedStructpathParse forbids parsing computed structpath strings in production +// code. Parsing re-validates the string, can panic on user keys with special characters +// and costs allocations; build the path from parts with structpath.NewPath, +// NewPathSlice or NewPattern instead. Parsing string literals and parsing in tests is fine. +func NoComputedStructpathParse(m dsl.Matcher) { + m.Match(`structpath.MustParsePath($s)`, `structpath.MustParsePattern($s)`). + Where(!m["s"].Const && !m.File().Name.Matches(`_test\.go$`) && !m.File().PkgPath.Matches(`internal/bundletest`)). + Report(`build computed paths with structpath.NewPath / NewPattern instead of parsing them`) + + m.Match(`structpath.MustParsePaths($*_, $s, $*_)`). + Where(!m["s"].Const && !m.File().Name.Matches(`_test\.go$`) && !m.File().PkgPath.Matches(`internal/bundletest`)). + Report(`build computed paths with structpath.NewPathSlice instead of parsing them`) + + m.Match( + `structpath.ParsePath(fmt.Sprintf($*_))`, + `structpath.ParsePath($a + $b)`, + `structpath.ParsePattern(fmt.Sprintf($*_))`, + `structpath.ParsePattern($a + $b)`, + ). + Where(!m.File().Name.Matches(`_test\.go$`)). + Report(`build the path with structpath.NewPath / NewPattern instead of formatting and parsing a string`) +} diff --git a/libs/structs/structpath/path.go b/libs/structs/structpath/path.go index bd0c33e4858..1f1e46465f8 100644 --- a/libs/structs/structpath/path.go +++ b/libs/structs/structpath/path.go @@ -369,6 +369,37 @@ func NewPatternBracketStar(prev *PatternNode) *PatternNode { }) } +// wildcard is a NewPattern part that matches any key or element. +type wildcard int + +// Wildcards for [NewPattern]: AnyKey matches any map key or field (rendered ".*") and +// AnyIndex matches any sequence element (rendered "[*]"). +const ( + AnyKey wildcard = tagDotStar + AnyIndex wildcard = tagBracketStar +) + +// NewPattern appends parts to prev like [NewPath]; AnyKey and AnyIndex append wildcards. +func NewPattern(prev *PatternNode, parts ...any) *PatternNode { + for _, part := range parts { + switch v := part.(type) { + case string: + prev = NewPatternStringKey(prev, v) + case int: + prev = NewPatternIndex(prev, v) + case wildcard: + if v == AnyKey { + prev = NewPatternDotStar(prev) + } else { + prev = NewPatternBracketStar(prev) + } + default: + panic(fmt.Sprintf("structpath.NewPattern: unsupported part %#v", part)) + } + } + return prev +} + func NewPatternKeyValue(prev *PatternNode, key, value string) *PatternNode { return (*PatternNode)(NewKeyValue((*PathNode)(prev), key, value)) } diff --git a/libs/structs/structpath/path_test.go b/libs/structs/structpath/path_test.go index 4e9901e6a46..6901d8381e1 100644 --- a/libs/structs/structpath/path_test.go +++ b/libs/structs/structpath/path_test.go @@ -1304,3 +1304,13 @@ func TestNewPathSlice(t *testing.T) { assert.Equal(t, "resources.jobs['${var.env}_job']", paths[0].String()) assert.Equal(t, "sync.paths[3]", NewPathSlice("sync", "paths", 3)[0].String()) } + +func TestNewPattern(t *testing.T) { + p := NewPattern(nil, "resources", "jobs", AnyKey, "tasks", AnyIndex, "libraries") + assert.Equal(t, MustParsePattern("resources.jobs.*.tasks[*].libraries").String(), p.String()) + + ext := NewPattern(p, AnyIndex, "whl", 0) + assert.Equal(t, "resources.jobs.*.tasks[*].libraries[*].whl[0]", ext.String()) + + assert.Panics(t, func() { NewPattern(nil, 1.5) }) +} From bcafaf05fff46119203e65d63249a5bd1393f4d7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 8 Oct 2026 13:48:43 +0200 Subject: [PATCH 2/2] config: GetLocationsOf/GetLocationOf/DefinitionLocationOf for path nodes Co-authored-by: Isaac --- .agents/rules/style-guide-go.md | 2 +- bundle/apps/validate.go | 12 +++++------ bundle/artifacts/prepare.go | 5 +++-- bundle/config/loader/process_root_includes.go | 3 ++- .../apply_source_linked_deployment_preset.go | 2 +- .../mutator/resolve_job_run_file_triggers.go | 21 ++++++++++--------- .../model_serving_endpoint_fixups.go | 5 +++-- .../resourcemutator/secret_scope_fixups.go | 2 +- bundle/config/mutator/sync_infer_root.go | 2 +- .../mutator/validate_job_run_triggers.go | 15 ++++++------- bundle/config/root.go | 7 ++++++- .../validate/job_cluster_key_defined.go | 2 +- .../config/validate/job_task_cluster_spec.go | 2 +- bundle/config/validate/required.go | 8 +++---- .../config/validate/validate_artifact_path.go | 2 +- .../validate/validate_deployment_fields.go | 2 +- .../validate_job_run_idempotency_token.go | 2 +- .../check_dashboards_modified_remotely.go | 2 +- bundle/deploy/metadata/compute.go | 7 ++++--- libs/gorules/rule_structpath_parse.go | 14 +++++++++++++ 20 files changed, 71 insertions(+), 46 deletions(-) diff --git a/.agents/rules/style-guide-go.md b/.agents/rules/style-guide-go.md index 07847641fdc..ad5c87c4888 100644 --- a/.agents/rules/style-guide-go.md +++ b/.agents/rules/style-guide-go.md @@ -119,7 +119,7 @@ return fieldPaths ### Structpath -**RULE: Build computed structpath paths and patterns from parts (`structpath.NewPath`, `NewPathSlice`, `NewPattern`); never format or concatenate a string to parse it.** `MustParsePath`/`MustParsePattern`/`MustParsePaths` are for string literals and tests; `ParsePath` with the error handled is for user input. The `NoComputedStructpathParse` ruleguard rule enforces this. +**RULE: Build computed structpath paths and patterns from parts (`structpath.NewPath`, `NewPathSlice`, `NewPattern`); never format or concatenate a string to parse it (for location lookups pass the node to `GetLocationsOf`).** `MustParsePath`/`MustParsePattern`/`MustParsePaths` are for string literals and tests; `ParsePath` with the error handled is for user input. The `NoComputedStructpathParse` ruleguard rule enforces this. ### Environment variables diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index d2b4355c635..be356a3d6d8 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -28,7 +28,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics Severity: diag.Error, Summary: "Missing app source code path or git source", Detail: fmt.Sprintf("app resource '%s' should have either source_code_path or git_source field", key), - Locations: b.Config.GetLocations("resources.apps." + key), + Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "apps", key)), }) continue } @@ -38,7 +38,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics Severity: diag.Error, Summary: "Both source_code_path and git_source fields are set", Detail: fmt.Sprintf("app resource '%s' should have either source_code_path or git_source field, not both", key), - Locations: b.Config.GetLocations("resources.apps." + key), + Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "apps", key)), }) continue } @@ -48,7 +48,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics Severity: diag.Error, Summary: "Duplicate app source code path", 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]), - Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)), + Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "apps", key, "source_code_path")), }) } usedSourceCodePaths[app.SourceCodePath] = key @@ -149,7 +149,7 @@ func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resourc continue } - appPath := "resources.apps." + appKey + appPath := structpath.NewPath(nil, "resources", "apps", appKey) diags = append(diags, diag.Diagnostic{ Severity: diag.Warning, 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 +167,8 @@ func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resourc ref.permission, appKey, ), - Paths: structpath.NewPathSlice("resources", "apps", appKey), - Locations: b.Config.GetLocations(appPath), + Paths: []*structpath.PathNode{appPath}, + Locations: b.Config.GetLocationsOf(appPath), }) } diff --git a/bundle/artifacts/prepare.go b/bundle/artifacts/prepare.go index d5330208eeb..bfd836121fa 100644 --- a/bundle/artifacts/prepare.go +++ b/bundle/artifacts/prepare.go @@ -17,6 +17,7 @@ import ( "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/logdiag" "github.com/databricks/cli/libs/python" + "github.com/databricks/cli/libs/structs/structpath" ) func Prepare() bundle.Mutator { @@ -38,7 +39,7 @@ func (m *prepare) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for _, artifactName := range slices.Sorted(maps.Keys(b.Config.Artifacts)) { artifact := b.Config.Artifacts[artifactName] if artifact == nil { - l := b.Config.GetLocation("artifacts." + artifactName) + l := b.Config.GetLocationOf(structpath.NewPath(nil, "artifacts", artifactName)) logdiag.LogDiag(ctx, diag.Diagnostic{ Severity: diag.Error, Summary: "Artifact not properly configured", @@ -61,7 +62,7 @@ func (m *prepare) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics logdiag.LogError(ctx, fmt.Errorf("artifact %q: a tgz artifact needs a `files` entry naming the output path", artifactName)) } - l := b.Config.DefinitionLocation("artifacts." + artifactName) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "artifacts", artifactName)) dirPath := filepath.Dir(l.File) // Check if source paths are absolute, if not, make them absolute diff --git a/bundle/config/loader/process_root_includes.go b/bundle/config/loader/process_root_includes.go index 36166fa8512..db942390c73 100644 --- a/bundle/config/loader/process_root_includes.go +++ b/bundle/config/loader/process_root_includes.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/logdiag" + "github.com/databricks/cli/libs/structs/structpath" ) type processRootIncludes struct{} @@ -112,7 +113,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. Summary: "Files in the 'include' configuration section must be YAML or JSON files.", Detail: fmt.Sprintf("The file %s in the 'include' configuration section is not a YAML or JSON file, and only such files are supported. To include files to sync, specify them in the 'sync.include' configuration section instead.", rel), // The match's index within the glob is unrelated to the entry's position in the include list. - Locations: b.Config.GetLocations(fmt.Sprintf("include[%d]", entryIndex)), + Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "include", entryIndex)), }) continue } diff --git a/bundle/config/mutator/apply_source_linked_deployment_preset.go b/bundle/config/mutator/apply_source_linked_deployment_preset.go index c4aa9bb51af..5c560db0116 100644 --- a/bundle/config/mutator/apply_source_linked_deployment_preset.go +++ b/bundle/config/mutator/apply_source_linked_deployment_preset.go @@ -81,7 +81,7 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle Summary: "workspace.file_path setting will be ignored in source-linked deployment mode", Detail: "In source-linked deployment files are not copied to the destination and resources use source files instead", Paths: []*structpath.PathNode{path}, - Locations: b.Config.GetLocations(path.String()), + Locations: b.Config.GetLocationsOf(path), }, ) } diff --git a/bundle/config/mutator/resolve_job_run_file_triggers.go b/bundle/config/mutator/resolve_job_run_file_triggers.go index 35cffd0dcbe..3e097afbb61 100644 --- a/bundle/config/mutator/resolve_job_run_file_triggers.go +++ b/bundle/config/mutator/resolve_job_run_file_triggers.go @@ -15,6 +15,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" libsync "github.com/databricks/cli/libs/sync" ) @@ -65,7 +66,7 @@ func (*resolveJobRunFileTriggers) Apply(ctx context.Context, b *bundle.Bundle) d if t.OnFileChange == nil { continue } - path := fmt.Sprintf("resources.job_runs.%s.lifecycle.triggers[%d].on_file_change", name, i) + path := structpath.NewPath(nil, "resources", "job_runs", name, "lifecycle", "triggers", i, "on_file_change") pattern, fingerprint, d := resolveFileTrigger(b, path, *t.OnFileChange, syncable) diags = diags.Extend(d) if !d.HasError() { @@ -105,7 +106,7 @@ func listSyncableRelPaths(ctx context.Context, b *bundle.Bundle) ([]string, erro return out, nil } -func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string) (string, string, diag.Diagnostics) { +func resolveFileTrigger(b *bundle.Bundle, loc *structpath.PathNode, pattern string, syncable []string) (string, string, diag.Diagnostics) { relPattern, diags := validateFileTriggerPattern(b, loc, pattern) if diags.HasError() { return "", "", diags @@ -119,7 +120,7 @@ func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("invalid pattern %q: %s", pattern, err), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) continue } @@ -131,7 +132,7 @@ func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("hash %q: %s", rel, err), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) continue } @@ -145,20 +146,20 @@ func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fileTriggerPrefix + fmt.Sprintf("no synced files match %q", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } return relPattern, hex.EncodeToString(h.Sum(nil)), diags } -func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, diag.Diagnostics) { +func validateFileTriggerPattern(b *bundle.Bundle, loc *structpath.PathNode, pattern string) (string, diag.Diagnostics) { var diags diag.Diagnostics // A double star looks recursive but path.Match treats it as two ordinary stars. if strings.Contains(pattern, "**") { return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("** in %q is not supported; use * for a single directory level", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } // Reject a genuinely absolute path; Join would otherwise silently reinterpret it @@ -170,7 +171,7 @@ func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("pattern %q must be relative to the defining YAML file", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } // NormalizePaths has already rewritten YAML-relative globs to be bundle-root @@ -182,7 +183,7 @@ func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("pattern %q is not under the sync root", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } relPattern = filepath.ToSlash(relPattern) @@ -191,7 +192,7 @@ func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("invalid pattern %q: %s", pattern, err), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } return relPattern, diags diff --git a/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go b/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go index 6c25146e6bd..47ad68d9bcc 100644 --- a/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go +++ b/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go @@ -5,6 +5,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go/service/serving" ) @@ -56,7 +57,7 @@ func (m *modelServingEndpointFixups) Apply(ctx context.Context, b *bundle.Bundle Summary: "Cannot use both served_models and served_entities", Detail: "Model serving endpoint cannot specify both served_models and served_entities at the same time.", Locations: []diag.Location{ - b.Config.GetLocation("resources.model_serving_endpoints." + key), + b.Config.GetLocationOf(structpath.NewPath(nil, "resources", "model_serving_endpoints", key)), }, }) continue @@ -71,7 +72,7 @@ func (m *modelServingEndpointFixups) Apply(ctx context.Context, b *bundle.Bundle Summary: "Using served_models is deprecated", Detail: "The served_models field is deprecated. Please use served_entities instead.", Locations: []diag.Location{ - b.Config.GetLocation("resources.model_serving_endpoints." + key + ".config.served_models"), + b.Config.GetLocationOf(structpath.NewPath(nil, "resources", "model_serving_endpoints", key, "config", "served_models")), }, }) diff --git a/bundle/config/mutator/resourcemutator/secret_scope_fixups.go b/bundle/config/mutator/resourcemutator/secret_scope_fixups.go index 468d324905d..ac51968ea59 100644 --- a/bundle/config/mutator/resourcemutator/secret_scope_fixups.go +++ b/bundle/config/mutator/resourcemutator/secret_scope_fixups.go @@ -140,7 +140,7 @@ func (m *secretScopeFixups) Apply(ctx context.Context, b *bundle.Bundle) diag.Di Summary: "Failed to collapse permissions for secret scope", Detail: err.Error(), Paths: []*structpath.PathNode{path}, - Locations: []diag.Location{b.Config.GetLocation(path.String())}, + Locations: []diag.Location{b.Config.GetLocationOf(path)}, }, } } diff --git a/bundle/config/mutator/sync_infer_root.go b/bundle/config/mutator/sync_infer_root.go index 75223780081..b0cc9f0e8fb 100644 --- a/bundle/config/mutator/sync_infer_root.go +++ b/bundle/config/mutator/sync_infer_root.go @@ -93,7 +93,7 @@ func (m *syncInferRoot) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno diags = append(diags, diag.Diagnostic{ Severity: diag.Error, Summary: fmt.Sprintf("invalid sync path %q", path), - Locations: b.Config.GetLocations(fmt.Sprintf("sync.paths[%d]", i)), + Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "sync", "paths", i)), Paths: []*structpath.PathNode{structpath.NewIndex(structpath.MustParsePath("sync.paths"), i)}, }) } diff --git a/bundle/config/mutator/validate_job_run_triggers.go b/bundle/config/mutator/validate_job_run_triggers.go index d33a7263bde..3d9df745261 100644 --- a/bundle/config/mutator/validate_job_run_triggers.go +++ b/bundle/config/mutator/validate_job_run_triggers.go @@ -2,11 +2,11 @@ package mutator import ( "context" - "fmt" "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" ) type validateJobRunTriggers struct{} @@ -27,12 +27,12 @@ func (*validateJobRunTriggers) Apply(_ context.Context, b *bundle.Bundle) diag.D continue } for i, t := range jr.Lifecycle.Triggers { - path := fmt.Sprintf("resources.job_runs.%s.lifecycle.triggers[%d]", name, i) + path := structpath.NewPath(nil, "resources", "job_runs", name, "lifecycle", "triggers", i) if t.OnBundleDeploy == nil && t.OnFileChange == nil { diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers entry must set on_bundle_deploy or on_file_change", - Locations: b.Config.GetLocations(path), + Locations: b.Config.GetLocationsOf(path), }) continue } @@ -40,7 +40,7 @@ func (*validateJobRunTriggers) Apply(_ context.Context, b *bundle.Bundle) diag.D diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers entry must set only one of on_bundle_deploy or on_file_change", - Locations: b.Config.GetLocations(path), + Locations: b.Config.GetLocationsOf(path), }) continue } @@ -48,20 +48,21 @@ func (*validateJobRunTriggers) Apply(_ context.Context, b *bundle.Bundle) diag.D diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers.on_bundle_deploy must be true when set", - Locations: b.Config.GetLocations(path + ".on_bundle_deploy"), + Locations: b.Config.GetLocationsOf(structpath.NewPath(path, "on_bundle_deploy")), }) } if t.OnFileChange != nil { + onFileChange := structpath.NewPath(path, "on_file_change") if strings.TrimSpace(*t.OnFileChange) == "" { diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers.on_file_change must be non-empty when set", - Locations: b.Config.GetLocations(path + ".on_file_change"), + Locations: b.Config.GetLocationsOf(onFileChange), }) continue } // Report bad patterns at validate time; hashing only runs on deploy. - _, patternDiags := validateFileTriggerPattern(b, path+".on_file_change", *t.OnFileChange) + _, patternDiags := validateFileTriggerPattern(b, onFileChange, *t.OnFileChange) diags = diags.Extend(patternDiags) } } diff --git a/bundle/config/root.go b/bundle/config/root.go index 07c2933eb6b..73e076ce6e7 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -589,7 +589,12 @@ func (r Root) DefinitionLocation(path string) diag.Location { if err != nil { return diag.Location{} } - locs := r.locations.At(p) + return r.DefinitionLocationOf(p) +} + +// DefinitionLocationOf is [Root.DefinitionLocation] for a path node. +func (r Root) DefinitionLocationOf(path *structpath.PathNode) diag.Location { + locs := r.locations.At(path) if len(locs) == 0 { return diag.Location{} } diff --git a/bundle/config/validate/job_cluster_key_defined.go b/bundle/config/validate/job_cluster_key_defined.go index 23f7f4161c0..14dacac59f5 100644 --- a/bundle/config/validate/job_cluster_key_defined.go +++ b/bundle/config/validate/job_cluster_key_defined.go @@ -61,7 +61,7 @@ func checkJobClusterKey(b *bundle.Bundle, jobClusterKeys map[string]bool, jobClu // Show only the location where the job_cluster_key is defined. // Other associated locations are not relevant since they are // overridden during merging. - Locations: b.Config.GetLocations(path.String()), + Locations: b.Config.GetLocationsOf(path), Paths: []*structpath.PathNode{path}, }} } diff --git a/bundle/config/validate/job_task_cluster_spec.go b/bundle/config/validate/job_task_cluster_spec.go index 56191d114b7..71bce7173d5 100644 --- a/bundle/config/validate/job_task_cluster_spec.go +++ b/bundle/config/validate/job_task_cluster_spec.go @@ -92,7 +92,7 @@ func validateJobTask(b *bundle.Bundle, task jobs.Task, taskPath *structpath.Path Severity: diag.Error, Summary: "Missing required cluster or environment settings", Detail: detail, - Locations: b.Config.GetLocations(taskPath.String()), + Locations: b.Config.GetLocationsOf(taskPath), Paths: []*structpath.PathNode{taskPath}, }) } diff --git a/bundle/config/validate/required.go b/bundle/config/validate/required.go index 3de90d4c335..2cad8a4d9be 100644 --- a/bundle/config/validate/required.go +++ b/bundle/config/validate/required.go @@ -96,11 +96,11 @@ func errorForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnosti diags := diag.Diagnostics{} for key, dashboard := range b.Config.Resources.Dashboards { if dashboard.DisplayName == "" { - nameLocations = append(nameLocations, b.Config.GetLocations("resources.dashboards."+key)...) + nameLocations = append(nameLocations, b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "dashboards", key))...) namePaths = append(namePaths, structpath.NewPath(nil, "resources", "dashboards", key)) } if dashboard.WarehouseId == "" { - warehouseIdLocations = append(warehouseIdLocations, b.Config.GetLocations("resources.dashboards."+key)...) + warehouseIdLocations = append(warehouseIdLocations, b.Config.GetLocationsOf(structpath.NewPath(nil, "resources", "dashboards", key))...) warehouseIdPaths = append(warehouseIdPaths, structpath.NewPath(nil, "resources", "dashboards", key)) } } @@ -130,7 +130,7 @@ func errorForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnosti diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "sql_warehouse name is required", - Locations: b.Config.GetLocations(path.String()), + Locations: b.Config.GetLocationsOf(path), Paths: []*structpath.PathNode{path}, }) } @@ -199,7 +199,7 @@ func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle Severity: diag.Error, Summary: "secret scope permission principal is required", Detail: "Set one of user_name, group_name or service_principal_name", - Locations: b.Config.GetLocations(scopePath.String()), + Locations: b.Config.GetLocationsOf(scopePath), Paths: []*structpath.PathNode{path}, }) } diff --git a/bundle/config/validate/validate_artifact_path.go b/bundle/config/validate/validate_artifact_path.go index 0989523d4a9..572c3a0de5a 100644 --- a/bundle/config/validate/validate_artifact_path.go +++ b/bundle/config/validate/validate_artifact_path.go @@ -69,7 +69,7 @@ func findVolumeInBundle(r config.Root, catalogName, schemaName, volumeName strin continue } volumePath := structpath.NewPath(nil, "resources", "volumes", k) - return volumePath, r.GetLocations(volumePath.String()), true + return volumePath, r.GetLocationsOf(volumePath), true } return nil, nil, false } diff --git a/bundle/config/validate/validate_deployment_fields.go b/bundle/config/validate/validate_deployment_fields.go index 10d95644d13..59868cb3fca 100644 --- a/bundle/config/validate/validate_deployment_fields.go +++ b/bundle/config/validate/validate_deployment_fields.go @@ -35,7 +35,7 @@ func (v *validateDeploymentFields) Apply(_ context.Context, b *bundle.Bundle) di Severity: diag.Error, Summary: field + " must not be set in bundle configuration; it is managed by Declarative Automation Bundles", Paths: []*structpath.PathNode{path}, - Locations: b.Config.GetLocations(path.String()), + Locations: b.Config.GetLocationsOf(path), }) } diff --git a/bundle/config/validate/validate_job_run_idempotency_token.go b/bundle/config/validate/validate_job_run_idempotency_token.go index 4ed6cc75694..61e07e302ca 100644 --- a/bundle/config/validate/validate_job_run_idempotency_token.go +++ b/bundle/config/validate/validate_job_run_idempotency_token.go @@ -37,7 +37,7 @@ func (v *validateJobRunIdempotencyToken) Apply(_ context.Context, b *bundle.Bund Severity: diag.Error, Summary: "idempotency_token must not be set in bundle configuration; the CLI sets it on each run-now request", Paths: []*structpath.PathNode{path}, - Locations: b.Config.GetLocations(path.String()), + Locations: b.Config.GetLocationsOf(path), }) } diff --git a/bundle/deploy/check_dashboards_modified_remotely.go b/bundle/deploy/check_dashboards_modified_remotely.go index 06e3e29f3a1..d685f7ebeb6 100644 --- a/bundle/deploy/check_dashboards_modified_remotely.go +++ b/bundle/deploy/check_dashboards_modified_remotely.go @@ -73,7 +73,7 @@ func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.B } path := structpath.NewPath(nil, "resources", "dashboards", dashboard.Name) - loc := b.Config.GetLocation(path.String()) + loc := b.Config.GetLocationOf(path) actual, err := b.WorkspaceClient(ctx).Lakeview.GetByDashboardId(ctx, dashboard.ID) if err != nil { diags = diags.Append(diag.Diagnostic{ diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index c8775748151..d4db537ded0 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle/metadata" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/structs/structpath" ) type compute struct{} @@ -46,7 +47,7 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for name, job := range b.Config.Resources.Jobs { // Compute config file path the job is defined in, relative to the bundle // root - l := b.Config.DefinitionLocation("resources.jobs." + name) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "resources", "jobs", name)) if l.File == "" { // Skip resources that exist only in the deployment state: statemgmt.Load, // which runs before this mutator, injects them into the config without a @@ -73,7 +74,7 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for name, pipeline := range b.Config.Resources.Pipelines { // Compute config file path the pipeline is defined in, relative to the bundle // root - l := b.Config.DefinitionLocation("resources.pipelines." + name) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "resources", "pipelines", name)) if l.File == "" { // Skip resources that exist only in the deployment state: statemgmt.Load, // which runs before this mutator, injects them into the config without a @@ -98,7 +99,7 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for name, dashboard := range b.Config.Resources.Dashboards { // Compute config file path the dashboard is defined in, relative to the bundle // root - l := b.Config.DefinitionLocation("resources.dashboards." + name) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "resources", "dashboards", name)) if l.File == "" { // Skip resources that exist only in the deployment state: statemgmt.Load, // which runs before this mutator, injects them into the config without a diff --git a/libs/gorules/rule_structpath_parse.go b/libs/gorules/rule_structpath_parse.go index 672aabd5a73..74b6ca6edd9 100644 --- a/libs/gorules/rule_structpath_parse.go +++ b/libs/gorules/rule_structpath_parse.go @@ -23,4 +23,18 @@ func NoComputedStructpathParse(m dsl.Matcher) { ). Where(!m.File().Name.Matches(`_test\.go$`)). Report(`build the path with structpath.NewPath / NewPattern instead of formatting and parsing a string`) + + m.Match( + `$c.GetLocations($a + $b)`, + `$c.GetLocation($a + $b)`, + `$c.DefinitionLocation($a + $b)`, + `$c.GetLocations(fmt.Sprintf($*_))`, + `$c.GetLocation(fmt.Sprintf($*_))`, + `$c.DefinitionLocation(fmt.Sprintf($*_))`, + `$c.GetLocations($p.String())`, + `$c.GetLocation($p.String())`, + `$c.DefinitionLocation($p.String())`, + ). + Where(!m.File().Name.Matches(`_test\.go$`)). + Report(`pass a *structpath.PathNode to GetLocationsOf / GetLocationOf / DefinitionLocationOf instead of a built string`) }