Skip to content

Commit bf1d217

Browse files
authored
Anchor alert file_path reads to the bundle root (#5488)
## Why Found during a full-repo review of the CLI. An alert's `file_path` is normalized to be relative to the bundle root, but the `.dbalert.json` file was read with `os.ReadFile`, which resolves relative paths against the process working directory. Running `bundle validate` or `bundle deploy` from a subdirectory of the bundle failed with "failed to read .dbalert.json file ...: no such file or directory", or could silently read an unrelated file that happens to match the path. Dashboards already handle this correctly by reading through the bundle's filesystem root. ## Changes Before, alert `.dbalert.json` files could only be loaded when the CLI ran from the bundle root; now they load from any directory within the bundle. `LoadDBAlertFiles` joins relative `file_path` values with `b.BundleRootPath` before reading. Absolute paths are kept as-is, matching how `NormalizePaths` leaves them untouched. Also fixed a malformed internal error message in the same function that wrapped a nil error with `%w` (printing `%!w(<nil>)`); it now reports the actual kind of the value. ## Test plan - [x] New unit test `TestLoadDBAlertFilesRelativeToBundleRoot` in `bundle/config/mutator/load_dbalert_files_test.go`; verified it fails without the fix (the test process working directory is not the bundle root) and passes with it - [x] New acceptance test `acceptance/bundle/resources/alerts/with_file_run_from_subdir` that runs `bundle validate` from both the bundle root and a subdirectory; verified it reproduces the bug without the fix and passes with it on both engines (terraform and direct) - [x] Existing alerts unit and acceptance tests pass (`go test ./bundle/config/mutator/`, `go test ./acceptance -run 'TestAccept/bundle/resources/alerts'`) - [x] `./task fmt-q`, `./task lint-q`, and `./task checks` pass This pull request and its description were written by Isaac.
1 parent ec9622d commit bf1d217

9 files changed

Lines changed: 167 additions & 2 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
bundle:
2+
name: alerts-with-file-run-from-subdir
3+
4+
include:
5+
- resources/*.yml

acceptance/bundle/resources/alerts/with_file_run_from_subdir/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: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
2+
=== validate from the bundle root
3+
>>> [CLI] bundle validate -o json
4+
{
5+
"custom_summary": "My alert from file",
6+
"display_name": "myalert",
7+
"evaluation": {
8+
"comparison_operator": "EQUAL",
9+
"notification": {
10+
"notify_on_ok": false,
11+
"retrigger_seconds": 1
12+
},
13+
"source": {
14+
"aggregation": "MAX",
15+
"display": "1",
16+
"name": "1"
17+
},
18+
"threshold": {
19+
"value": {
20+
"double_value": 2
21+
}
22+
}
23+
},
24+
"file_path": "resources/alert.dbalert.json",
25+
"parent_path": "/Workspace/Users/[USERNAME]/.bundle/alerts-with-file-run-from-subdir/default/resources",
26+
"query_text": "select 2\n",
27+
"schedule": {
28+
"pause_status": "UNPAUSED",
29+
"quartz_cron_schedule": "44 19 */1 * * ?",
30+
"timezone_id": "Europe/Amsterdam"
31+
},
32+
"warehouse_id": "[NUMID]"
33+
}
34+
35+
=== validate from a subdirectory of the bundle
36+
>>> [CLI] bundle validate -o json
37+
{
38+
"custom_summary": "My alert from file",
39+
"display_name": "myalert",
40+
"evaluation": {
41+
"comparison_operator": "EQUAL",
42+
"notification": {
43+
"notify_on_ok": false,
44+
"retrigger_seconds": 1
45+
},
46+
"source": {
47+
"aggregation": "MAX",
48+
"display": "1",
49+
"name": "1"
50+
},
51+
"threshold": {
52+
"value": {
53+
"double_value": 2
54+
}
55+
}
56+
},
57+
"file_path": "resources/alert.dbalert.json",
58+
"parent_path": "/Workspace/Users/[USERNAME]/.bundle/alerts-with-file-run-from-subdir/default/resources",
59+
"query_text": "select 2\n",
60+
"schedule": {
61+
"pause_status": "UNPAUSED",
62+
"quartz_cron_schedule": "44 19 */1 * * ?",
63+
"timezone_id": "Europe/Amsterdam"
64+
},
65+
"warehouse_id": "[NUMID]"
66+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
"custom_summary": "My alert from file",
3+
"evaluation": {
4+
"comparison_operator": "EQUAL",
5+
"notification": {
6+
"notify_on_ok": false,
7+
"retrigger_seconds": 1
8+
},
9+
"source": {
10+
"aggregation": "MAX",
11+
"display": "1",
12+
"name": "1"
13+
},
14+
"threshold": {
15+
"value": {
16+
"double_value": 2
17+
}
18+
}
19+
},
20+
"query_lines": [
21+
"select 2"
22+
],
23+
"schedule": {
24+
"pause_status": "UNPAUSED",
25+
"quartz_cron_schedule": "44 19 */1 * * ?",
26+
"timezone_id": "Europe/Amsterdam"
27+
}
28+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
resources:
2+
alerts:
3+
myalert:
4+
warehouse_id: "0123456789012345"
5+
display_name: "myalert"
6+
file_path: ./alert.dbalert.json
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
title "validate from the bundle root"
2+
trace $CLI bundle validate -o json | jq .resources.alerts.myalert
3+
4+
title "validate from a subdirectory of the bundle"
5+
cd resources
6+
trace $CLI bundle validate -o json | jq .resources.alerts.myalert
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Local = true
2+
RecordRequests = false
3+
Ignore = [".databricks"]

bundle/config/mutator/load_dbalert_files.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"os"
8+
"path/filepath"
89
"slices"
910
"strings"
1011

@@ -63,7 +64,7 @@ func (m *loadDBAlertFiles) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
6364
// No other fields other than allowedInYAML should be set in the bundle YAML.
6465
m, ok := alertV.AsMap()
6566
if !ok {
66-
return diag.FromErr(fmt.Errorf("internal error: alert value is not a map: %w", err))
67+
return diag.FromErr(fmt.Errorf("internal error: alert value is not a map, got %s", alertV.Kind()))
6768
}
6869

6970
for _, p := range m.Pairs() {
@@ -90,7 +91,15 @@ func (m *loadDBAlertFiles) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
9091
}
9192
}
9293

93-
content, err := os.ReadFile(alert.FilePath)
94+
// NormalizePaths rewrote file_path to be relative to the bundle root, but the
95+
// process working directory can be any directory within the bundle, so anchor
96+
// the read to the bundle root. Absolute paths are kept as-is by NormalizePaths.
97+
filePath := alert.FilePath
98+
if !filepath.IsAbs(filePath) {
99+
filePath = filepath.Join(b.BundleRootPath, filePath)
100+
}
101+
102+
content, err := os.ReadFile(filePath)
94103
if err != nil {
95104
return diag.Diagnostics{
96105
{

bundle/config/mutator/load_dbalert_files_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,42 @@ func TestLoadDBAlertFiles(t *testing.T) {
7979
assert.Equal(t, "abc123", b.Config.Resources.Alerts["my_alert"].WarehouseId)
8080
assert.Equal(t, "SELECT * FROM table\nWHERE value > 100\n", b.Config.Resources.Alerts["my_alert"].QueryText)
8181
}
82+
83+
func TestLoadDBAlertFilesRelativeToBundleRoot(t *testing.T) {
84+
dir := t.TempDir()
85+
86+
alertJSON := `{
87+
"query_lines": [
88+
"SELECT 1"
89+
]
90+
}`
91+
92+
err := os.WriteFile(filepath.Join(dir, "alert.dbalert.json"), []byte(alertJSON), 0o644)
93+
require.NoError(t, err)
94+
95+
b := &bundle.Bundle{
96+
BundleRootPath: dir,
97+
Config: config.Root{
98+
Resources: config.Resources{
99+
Alerts: map[string]*resources.Alert{
100+
"my_alert": {
101+
FilePath: "alert.dbalert.json",
102+
AlertV2: sql.AlertV2{
103+
DisplayName: "Test Alert",
104+
WarehouseId: "abc123",
105+
},
106+
},
107+
},
108+
},
109+
},
110+
}
111+
112+
bundletest.SetLocation(b, "resources.alerts.my_alert", []dyn.Location{{
113+
File: filepath.Join(dir, "databricks.yml"),
114+
}})
115+
116+
diags := bundle.Apply(t.Context(), b, mutator.LoadDBAlertFiles())
117+
require.NoError(t, diags.Error())
118+
119+
assert.Equal(t, "SELECT 1\n", b.Config.Resources.Alerts["my_alert"].QueryText)
120+
}

0 commit comments

Comments
 (0)