Skip to content

Commit 2f1627d

Browse files
Error when bundle root path contains glob (#3096)
## Changes This PR adds an error when the bundle root path contains a glob character. ``` Error: Bundle root path contains glob pattern characters The path to the bundle root [TEST_TMP_DIR]/[abc] contains glob pattern character "[". Please remove the character from this path to use bundle commands. Found 1 error ``` ## Why We provide full paths to `filepath.Glob` to figure out which paths to include. If the file path itself happens to contain a glob pattern, then `filepath.Glob` interprets that as a glob pattern, even though we should not be parsing glob patterns there. Escaping those glob patterns is also not possible because escaping is not supported on windows. ref: https://pkg.go.dev/path/filepath#Match Thus, we error here to inform users to modify their root paths before doing a bundle deployment. Note: 1. We could support this but that would mean writing our own glob function. We can use the `filepath.Match` function to match the patterns themselves but we would have to walk the paths. The ROI for this option is low at the moment because our custom function would have to have the same semantics as the `filepath.Glob` function which would take some time to get right. 2. This PR does not completely solve the problem since there are some other places as well where glob patterns might be accidentally parsed. However, this PR addresses the most important usecase i.e. making during the `includes` are correct and complete. Other usecases like libraries not being uploaded correctly are less important and even those will be very likely caught by this PR unless the user only has a single `databricks.yml` in their bundle. refs: https://github.com/databricks/cli/blob/d177292b5b2e5ce779c5d6c5d42feeec00cc533a/bundle/artifacts/expand_globs.go#L68 https://github.com/databricks/cli/blob/d177292b5b2e5ce779c5d6c5d42feeec00cc533a/bundle/libraries/expand_glob_references.go#L42 ## Tests New unit and acceptance tests.
1 parent 3812c9a commit 2f1627d

File tree

8 files changed

+119
-0
lines changed

8 files changed

+119
-0
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Release v0.259.0
44

55
### Notable Changes
6+
* Error when the absolute path to `databricks.yml` contains a glob character. These are: `*`, `?`, `[`, `]` and `^`. If the path to the `databricks.yml` file on your local filesystem contains one of these characters, that could lead to incorrect computation of glob patterns for the `includes` block and might cause resources to be deleted. After this patch users will not be at risk for unexpected deletions due to this issue. ([#3096](https://github.com/databricks/cli/pull/3096))
67
* Diagnostics messages are no longer buffered to be printed at the end of command, flushed after every mutator ([#3175](https://github.com/databricks/cli/pull/3175))
78
* Diagnostics are now always rendered with forward slashes in file paths, even on Windows ([#3175](https://github.com/databricks/cli/pull/3175))
89
* "bundle summary" now prints diagnostics to stderr instead of stdout in text output mode ([#3175](https://github.com/databricks/cli/pull/3175))

acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml

Whitespace-only changes.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Local = true
2+
Cloud = false
3+
4+
[EnvMatrix]
5+
DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct-exp"]
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
2+
>>> [CLI] bundle validate
3+
Error: Bundle root path contains glob pattern characters
4+
5+
The path to the bundle root [TEST_TMP_DIR]/[abc] contains glob pattern character "[". Please remove the character from this path to use bundle commands.
6+
7+
8+
Found 1 error
9+
10+
Exit code: 1
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
cd [abc]
2+
trace $CLI bundle validate
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[[Repls]]
2+
Old = '\\'
3+
New = '/'

bundle/config/loader/process_root_includes.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,18 @@ func (m *processRootIncludes) Name() string {
2424
return "ProcessRootIncludes"
2525
}
2626

27+
// hasGlobCharacters checks if a path contains any glob characters.
28+
func hasGlobCharacters(path string) (string, bool) {
29+
// List of glob characters supported by the filepath package in [filepath.Match]
30+
globCharacters := []string{"*", "?", "[", "]", "^"}
31+
for _, char := range globCharacters {
32+
if strings.Contains(path, char) {
33+
return char, true
34+
}
35+
}
36+
return "", false
37+
}
38+
2739
func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
2840
var out []bundle.Mutator
2941

@@ -39,6 +51,24 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
3951
var files []string
4052
var diags diag.Diagnostics
4153

54+
// We error on glob characters in the bundle root path since they are
55+
// parsed by [filepath.Glob] as being glob patterns and thus can cause
56+
// unexpected behavior.
57+
//
58+
// The standard library does not support globbing relative paths from a specified
59+
// base directory. To support this, we can either:
60+
// 1. Change CWD to the bundle root path before calling [filepath.Glob]
61+
// 2. Implement our own custom globbing function. We can use [filepath.Match] to do so.
62+
if char, ok := hasGlobCharacters(b.BundleRootPath); ok {
63+
diags = diags.Append(diag.Diagnostic{
64+
Severity: diag.Error,
65+
Summary: "Bundle root path contains glob pattern characters",
66+
Detail: fmt.Sprintf("The path to the bundle root %s contains glob pattern character %q. Please remove the character from this path to use bundle commands.", b.BundleRootPath, char),
67+
})
68+
69+
return diags
70+
}
71+
4272
// For each glob, find all files to load.
4373
// Ordering of the list of globs is maintained in the output.
4474
// For matches that appear in multiple globs, only the first is kept.

bundle/config/loader/process_root_includes_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/databricks/cli/bundle/config"
1010
"github.com/databricks/cli/bundle/config/loader"
1111
"github.com/databricks/cli/internal/testutil"
12+
"github.com/databricks/cli/libs/diag"
1213
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
1415
)
@@ -111,3 +112,70 @@ func TestProcessRootIncludesNotExists(t *testing.T) {
111112
require.True(t, diags.HasError())
112113
assert.ErrorContains(t, diags.Error(), "notexist.yml defined in 'include' section does not match any files")
113114
}
115+
116+
func TestProcessRootIncludesGlobInRootPath(t *testing.T) {
117+
tests := []struct {
118+
name string
119+
root string
120+
diag diag.Diagnostic
121+
}{
122+
{
123+
name: "star",
124+
root: "foo/a*",
125+
diag: diag.Diagnostic{
126+
Severity: diag.Error,
127+
Summary: "Bundle root path contains glob pattern characters",
128+
Detail: `The path to the bundle root foo/a* contains glob pattern character "*". Please remove the character from this path to use bundle commands.`,
129+
},
130+
},
131+
{
132+
name: "question mark",
133+
root: "bar/?b",
134+
diag: diag.Diagnostic{
135+
Severity: diag.Error,
136+
Summary: "Bundle root path contains glob pattern characters",
137+
Detail: `The path to the bundle root bar/?b contains glob pattern character "?". Please remove the character from this path to use bundle commands.`,
138+
},
139+
},
140+
{
141+
name: "left bracket",
142+
root: "[ab",
143+
diag: diag.Diagnostic{
144+
Severity: diag.Error,
145+
Summary: "Bundle root path contains glob pattern characters",
146+
Detail: `The path to the bundle root [ab contains glob pattern character "[". Please remove the character from this path to use bundle commands.`,
147+
},
148+
},
149+
{
150+
name: "right bracket",
151+
root: "ab]/bax",
152+
diag: diag.Diagnostic{
153+
Severity: diag.Error,
154+
Summary: "Bundle root path contains glob pattern characters",
155+
Detail: `The path to the bundle root ab]/bax contains glob pattern character "]". Please remove the character from this path to use bundle commands.`,
156+
},
157+
},
158+
{
159+
name: "hat",
160+
root: "ab^bax",
161+
diag: diag.Diagnostic{
162+
Severity: diag.Error,
163+
Summary: "Bundle root path contains glob pattern characters",
164+
Detail: `The path to the bundle root ab^bax contains glob pattern character "^". Please remove the character from this path to use bundle commands.`,
165+
},
166+
},
167+
}
168+
169+
for _, test := range tests {
170+
t.Run(test.name, func(t *testing.T) {
171+
b := &bundle.Bundle{
172+
BundleRootPath: test.root,
173+
}
174+
175+
diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
176+
require.True(t, diags.HasError())
177+
assert.Len(t, diags, 1)
178+
assert.Equal(t, test.diag, diags[0])
179+
})
180+
}
181+
}

0 commit comments

Comments
 (0)