Support .sql files in file_path for alerts

.nextchanges/bundles/alert-sql-file-path.md

@@ -0,0 +1 @@
+* Support `.sql` files in an alert's `file_path`, so `query_text` can live in its own SQL file while other fields stay in the bundle configuration. ([#7330](https://github.com/databricks/cli/pull/7330))

acceptance/bundle/resources/alerts/with_sql_file/databricks.yml.tmpl

@@ -0,0 +1,26 @@
+bundle:
+  name: alerts-with-sql-file-$UNIQUE_NAME
+
+resources:
+  alerts:
+    myalert:
+      custom_summary: "My alert"
+      display_name: "My alert"
+      evaluation:
+        comparison_operator: "EQUAL"
+        notification:
+          notify_on_ok: false
+          retrigger_seconds: 1
+        source:
+          aggregation: "MAX"
+          display: "2"
+          name: "2"
+        threshold:
+          value:
+            double_value: 2
+      file_path: ./sql/failed_runs.sql
+      schedule:
+        pause_status: "UNPAUSED"
+        quartz_cron_schedule: "44 19 */1 * * ?"
+        timezone_id: "Europe/Amsterdam"
+      warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID

acceptance/bundle/resources/alerts/with_sql_file/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = true
+EnvMatrix.DMS = ["", "true"]

acceptance/bundle/resources/alerts/with_sql_file/output.txt

@@ -0,0 +1,52 @@
+
+>>> [CLI] bundle validate -o json
+{
+  "display_name": "My alert",
+  "file_path": "sql/failed_runs.sql",
+  "query_text": "-- Alert query kept in its own file so it can be reviewed and linted as SQL.\nselect 2\n",
+  "custom_summary": "My alert"
+}
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/alerts-with-sql-file-[UNIQUE_NAME]/default/files...
+Created alerts.myalert
+Files: 3 uploaded, 0 deleted
+Resources: 1 created, 0 changed, 0 deleted, 0 unchanged
+
+>>> [CLI] alerts-v2 get-alert [ALERT_ID]
+{
+  "display_name": "My alert",
+  "lifecycle_state": "ACTIVE",
+  "custom_summary": "My alert",
+  "query_text": "-- Alert query kept in its own file so it can be reviewed and linted as SQL.\nselect 2\n",
+  "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]"
+}
+
+=== assert that no permanent drift happens
+>>> [CLI] bundle plan
+Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
+
+=== edit the .sql file and redeploy
+>>> [CLI] bundle plan
+update alerts.myalert
+
+Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/alerts-with-sql-file-[UNIQUE_NAME]/default/files...
+Updated alerts.myalert
+Files: 1 uploaded, 0 deleted
+Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
+
+>>> [CLI] alerts-v2 get-alert [ALERT_ID]
+{
+  "query_text": "select 3\n"
+}
+
+>>> [CLI] bundle destroy --auto-approve
+The following resources will be deleted:
+  delete resources.alerts.myalert
+
+All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/alerts-with-sql-file-[UNIQUE_NAME]/default
+
+Destroy: 1 deleted

acceptance/bundle/resources/alerts/with_sql_file/script

@@ -0,0 +1,27 @@
+envsubst < databricks.yml.tmpl > databricks.yml
+
+cleanup() {
+    trace $CLI bundle destroy --auto-approve
+}
+trap cleanup EXIT
+
+trace $CLI bundle validate -o json | jq '.resources.alerts.myalert | {display_name, file_path, query_text, custom_summary}'
+
+trace $CLI bundle deploy
+
+alert_id=$($CLI bundle summary --output json | jq -r '.resources.alerts.myalert.id')
+
+add_repl "$alert_id" ALERT_ID
+
+trace $CLI alerts-v2 get-alert $alert_id | jq '{display_name, lifecycle_state, custom_summary, query_text, warehouse_id}'
+
+title "assert that no permanent drift happens"
+trace $CLI bundle plan
+
+title "edit the .sql file and redeploy"
+cat > sql/failed_runs.sql <<SQL
+select 3
+SQL
+trace $CLI bundle plan
+trace $CLI bundle deploy
+trace $CLI alerts-v2 get-alert $alert_id | jq '{query_text}'

acceptance/bundle/resources/alerts/with_sql_file/sql/failed_runs.sql

@@ -0,0 +1,2 @@
+-- Alert query kept in its own file so it can be reviewed and linted as SQL.
+select 2

acceptance/bundle/resources/alerts/with_sql_file/test.toml

@@ -0,0 +1,19 @@
+Cloud = true
+RecordRequests = false
+Ignore = [".databricks"]
+
+# Alert tests timeout during bundle deploy (hang at file upload for 50+ minutes).
+# Use aggressive 5-minute timeout until the issue is resolved.
+# See: https://github.com/databricks/cli/issues/4221
+TimeoutCloud = "5m"
+
+# MSYS2 automatically converts absolute paths like /Users/$username/$UNIQUE_NAME to
+# C:/Program Files/Git/Users/$username/UNIQUE_NAME before passing it to the CLI
+# Setting this environment variable prevents that conversion on windows.
+Env.MSYS_NO_PATHCONV = "1"
+
+# redact ?[ow]=[WORKSPACE_ID]. Different clouds can have different URL serialization, like [DATABRICKS_URL]/sql/alerts-v2/[ALERT_ID] vs [DATABRICKS_URL]/sql/alerts-v2/[ALERT_ID]?o=[WORKSPACE_ID].
+[[Repls]]
+Old = '\?[ow]=\[WORKSPACE_ID\]'
+New = ''
+Order = 1000

acceptance/bundle/resources/alerts/with_sql_file_query_text_error/databricks.yml

@@ -0,0 +1,11 @@
+bundle:
+  name: alerts-sql-file-query-text
+
+resources:
+  alerts:
+    myalert:
+      warehouse_id: "0123456789012345"
+      display_name: "My alert"
+      file_path: ./query.sql
+      # query_text comes from query.sql, so it cannot also be set here
+      query_text: "select 3"

acceptance/bundle/resources/alerts/with_sql_file_query_text_error/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+EnvMatrix.DMS = ["", "true"]

acceptance/bundle/resources/alerts/with_sql_file_query_text_error/output.txt

@@ -0,0 +1,15 @@
+
+>>> [CLI] bundle validate
+Error: query_text cannot be set when file_path points to a .sql file
+  at resources.alerts.myalert.query_text
+  in databricks.yml:11:19
+
+The alert query is read from query.sql. Remove either query_text or file_path.
+
+Name: alerts-sql-file-query-text
+Target: default
+Workspace:
+  User: [USERNAME]
+  Path: /Workspace/Users/[USERNAME]/.bundle/alerts-sql-file-query-text/default
+
+Found 1 error

acceptance/bundle/resources/alerts/with_sql_file_query_text_error/query.sql

@@ -0,0 +1 @@
+select 2

acceptance/bundle/resources/alerts/with_sql_file_query_text_error/script

@@ -0,0 +1 @@
+musterr trace $CLI bundle validate

acceptance/bundle/resources/alerts/with_sql_file_query_text_error/test.toml

@@ -0,0 +1,2 @@
+RecordRequests = false
+Ignore = [".databricks"]

bundle/config/mutator/load_dbalert_files.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/dyn/dynvar"
@@ -62,6 +63,13 @@ func (m *loadDBAlertFiles) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
 			return diag.FromErr(err)
 		}
 
+		if isAlertQueryFile(alert.FilePath) {
+			if diags := loadAlertQueryFile(b, alertKey, alert, alertV); diags.HasError() {
+				return diags
+			}
+			continue
+		}
+
 		// No other fields other than allowedInYAML should be set in the bundle YAML.
 		m, ok := alertV.AsMap()
 		if !ok {
@@ -92,15 +100,7 @@ func (m *loadDBAlertFiles) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
 			}
 		}
 
-		// NormalizePaths rewrote file_path to be relative to the bundle root, but the
-		// process working directory can be any directory within the bundle, so anchor
-		// the read to the bundle root. Absolute paths are kept as-is by NormalizePaths.
-		filePath := alert.FilePath
-		if !filepath.IsAbs(filePath) {
-			filePath = filepath.Join(b.BundleRootPath, filePath)
-		}
-
-		content, err := os.ReadFile(filePath)
+		content, err := os.ReadFile(alertFilePath(b, alert.FilePath))
 		if err != nil {
 			return diag.Diagnostics{
 				{
@@ -200,3 +200,86 @@ func (m *loadDBAlertFiles) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
 
 	return nil
 }
+
+// alertFilePath anchors a file_path to the bundle root. NormalizePaths rewrote
+// file_path to be relative to the bundle root, but the process working directory
+// can be any directory within the bundle. Absolute paths are kept as-is by
+// NormalizePaths.
+func alertFilePath(b *bundle.Bundle, filePath string) string {
+	if filepath.IsAbs(filePath) {
+		return filePath
+	}
+	return filepath.Join(b.BundleRootPath, filePath)
+}
+
+// isAlertQueryFile reports whether file_path points to a .sql file holding only
+// the alert query, as opposed to a full .dbalert.json alert definition.
+func isAlertQueryFile(filePath string) bool {
+	return strings.EqualFold(filepath.Ext(filePath), ".sql")
+}
+
+// loadAlertQueryFile sets query_text from a .sql file. Unlike a .dbalert.json, the
+// file only holds the query, so all other alert fields stay in the bundle YAML.
+func loadAlertQueryFile(b *bundle.Bundle, alertKey string, alert *resources.Alert, alertV dyn.Value) diag.Diagnostics {
+	filePathV := alertV.Get("file_path")
+	filePathPaths := structpath.NewPathSlice("resources", "alerts", alertKey, "file_path")
+
+	if q := alertV.Get("query_text"); q.Kind() != dyn.KindInvalid && q.Kind() != dyn.KindNil {
+		return diag.Diagnostics{
+			{
+				ID:        "",
+				Severity:  diag.Error,
+				Summary:   "query_text cannot be set when file_path points to a .sql file",
+				Detail:    fmt.Sprintf("The alert query is read from %s. Remove either query_text or file_path.", alert.FilePath),
+				Paths:     structpath.NewPathSlice("resources", "alerts", alertKey, "query_text"),
+				Locations: q.Locations(),
+			},
+		}
+	}
+
+	content, err := os.ReadFile(alertFilePath(b, alert.FilePath))
+	if err != nil {
+		return diag.Diagnostics{
+			{
+				ID:        "",
+				Severity:  diag.Error,
+				Summary:   fmt.Sprintf("failed to read alert query file %s: %s", alert.FilePath, err),
+				Detail:    "",
+				Paths:     filePathPaths,
+				Locations: filePathV.Locations(),
+			},
+		}
+	}
+
+	query := string(content)
+	if strings.TrimSpace(query) == "" {
+		return diag.Diagnostics{
+			{
+				ID:        "",
+				Severity:  diag.Error,
+				Summary:   fmt.Sprintf("alert query file %s is empty", alert.FilePath),
+				Detail:    "",
+				Paths:     filePathPaths,
+				Locations: filePathV.Locations(),
+			},
+		}
+	}
+
+	// The file is read after variables are resolved, so references in it would be
+	// sent to the backend verbatim. Reject them, like for .dbalert.json files.
+	if dynvar.ContainsVariableReference(query) {
+		return diag.Diagnostics{
+			{
+				ID:        "",
+				Severity:  diag.Error,
+				Summary:   fmt.Sprintf("alert query file %s must not contain variable interpolations.", alert.FilePath),
+				Detail:    "Please set query_text inline in the bundle configuration to use variables",
+				Paths:     filePathPaths,
+				Locations: filePathV.Locations(),
+			},
+		}
+	}
+
+	alert.QueryText = query
+	return nil
+}