diff --git a/package.json b/package.json index a415ff50d5..1ea9cc4e44 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "selfhost:postgres:migrate": "tsx scripts/migrate-selfhost-sqlite-to-postgres.ts", "selfhost:env-reference": "node scripts/gen-selfhost-env-reference.mjs", "selfhost:env-reference:check": "node scripts/gen-selfhost-env-reference.mjs --check", + "selfhost:validate-observability": "node scripts/validate-observability-configs.mjs", "cf-typegen": "wrangler types && perl -pi -e 's/[[:blank:]]+$//' worker-configuration.d.ts", "cf-typegen:check": "wrangler types --check", "db:migrate:local": "wrangler d1 migrations apply gittensory --local", @@ -66,7 +67,7 @@ "test:smoke:observability": "node scripts/smoke-observability-traces.mjs", "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node scripts/smoke-ui-browser.mjs", - "test:ci": "git diff --check && npm run actionlint && npm run db:migrations:check && npm run selfhost:env-reference:check && npm run cf-typegen:check && npm run typecheck && npm run test:coverage && npm run test:workers && npm run build:mcp && npm run test:mcp-pack && npm run build:miner && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run db:migrations:check && npm run selfhost:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run typecheck && npm run test:coverage && npm run test:workers && npm run build:mcp && npm run test:mcp-pack && npm run build:miner && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci && npm run changelog:check:mcp", "test:watch": "vitest", diff --git a/scripts/validate-observability-configs.d.mts b/scripts/validate-observability-configs.d.mts new file mode 100644 index 0000000000..89b17aad3a --- /dev/null +++ b/scripts/validate-observability-configs.d.mts @@ -0,0 +1,3 @@ +export declare function validateDashboards(dir: string): string[]; + +export declare function validateAlertRules(path: string): string[]; diff --git a/scripts/validate-observability-configs.mjs b/scripts/validate-observability-configs.mjs new file mode 100644 index 0000000000..a3d57c13b7 --- /dev/null +++ b/scripts/validate-observability-configs.mjs @@ -0,0 +1,149 @@ +#!/usr/bin/env node +// Validate Grafana dashboard JSON and Prometheus alert-rule YAML syntax + basic shape (#1943 deliverable: +// "Add validation for dashboard JSON / alert rule syntax"). Catches a broken JSON/YAML file or a +// structurally malformed dashboard/rule before it silently fails to load in the running stack -- Grafana +// and Prometheus both fail OPEN on a malformed file (skip it, log a warning), so nothing else would catch +// this until an operator notices a panel or alert is simply missing. +// +// The alert-rule `expr` check is a lightweight sanity check (balanced brackets, no dangling binary +// operator), NOT a real PromQL parser -- this repo has no promtool/PromQL-grammar dependency, and adding +// one is out of scope for this "if available" deliverable. It catches the obvious copy-paste/typo class +// of mistake; it does not validate PromQL semantics (unknown functions, wrong label matchers, etc.). +import { readFileSync, readdirSync } from "node:fs"; +import { pathToFileURL } from "node:url"; +import { parse as parseYaml } from "yaml"; + +// Valid JSON/YAML can parse to a non-object (null, a string, a number, an array of non-objects) -- +// dereferencing a property on that crashes instead of producing a validation error. Every dereference +// below goes through this guard first. +function isObject(value) { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +const BINARY_OPERATORS = ["==", "!=", ">=", "<=", ">", "<", "+", "-", "*", "/", "%", "^", "and", "or", "unless"]; + +// A deliberately lightweight PromQL sanity check -- NOT a real parser (no promtool dependency; see the +// module doc comment). Catches the class of mistake a copy-paste/typo produces: unbalanced brackets, or +// an expression left dangling on a binary operator with no right-hand side (e.g. "up =="). +function promqlSanityIssue(expr) { + const trimmed = expr.trim(); + // A stack, not a depth counter: a depth counter only checks NESTING COUNT, so "sum(foo[5m))" -- opened + // with "(" and "[", closed with ")" and ")" -- reaches depth 0 and would wrongly look balanced. The + // stack checks each closer against the delimiter it's actually supposed to match. + const pairs = { ")": "(", "]": "[", "}": "{" }; + const opens = new Set(["(", "[", "{"]); + const stack = []; + for (const ch of trimmed) { + if (opens.has(ch)) { + stack.push(ch); + } else if (ch in pairs) { + const top = stack.pop(); + if (top !== pairs[ch]) { + return `unbalanced brackets (unexpected "${ch}"${top ? `, expected the match for "${top}"` : ""})`; + } + } + } + if (stack.length > 0) return `unbalanced brackets (unclosed "${stack[stack.length - 1]}")`; + const lastToken = trimmed.split(/\s+/).pop() ?? ""; + if (BINARY_OPERATORS.includes(lastToken)) { + return `expression ends in the binary operator "${lastToken}" with no right-hand side`; + } + return null; +} + +export function validateDashboards(dir) { + const errors = []; + let files; + try { + files = readdirSync(dir).filter((f) => f.endsWith(".json")); + } catch (error) { + return [`${dir}: could not read directory — ${error.message}`]; + } + if (files.length === 0) errors.push(`${dir}: no dashboard JSON files found`); + for (const file of files) { + const path = `${dir}/${file}`; + let dashboard; + try { + dashboard = JSON.parse(readFileSync(path, "utf8")); + } catch (error) { + errors.push(`${path}: invalid JSON — ${error.message}`); + continue; + } + if (!isObject(dashboard)) { + errors.push(`${path}: top level must be a JSON object, not ${JSON.stringify(dashboard)}`); + continue; + } + if (typeof dashboard.title !== "string" || !dashboard.title) { + errors.push(`${path}: missing a non-empty top-level "title"`); + } + if (!Array.isArray(dashboard.panels)) { + errors.push(`${path}: missing a top-level "panels" array`); + } + } + return errors; +} + +export function validateAlertRules(path) { + const errors = []; + let doc; + try { + doc = parseYaml(readFileSync(path, "utf8")); + } catch (error) { + return [`${path}: invalid YAML — ${error.message}`]; + } + if (!Array.isArray(doc?.groups)) { + return [`${path}: missing a top-level "groups" array`]; + } + for (const [groupIndex, group] of doc.groups.entries()) { + if (!isObject(group)) { + errors.push(`${path}: groups[${groupIndex}] must be an object, not ${JSON.stringify(group)}`); + continue; + } + if (typeof group.name !== "string" || !group.name) { + errors.push(`${path}: a group is missing a non-empty "name"`); + } + if (!Array.isArray(group.rules)) { + errors.push(`${path}: group "${group.name ?? "?"}" is missing a "rules" array`); + continue; + } + for (const [ruleIndex, rule] of group.rules.entries()) { + if (!isObject(rule)) { + errors.push( + `${path}: group "${group.name}" rules[${ruleIndex}] must be an object, not ${JSON.stringify(rule)}`, + ); + continue; + } + const label = rule.alert ?? "(unnamed rule)"; + if (typeof rule.alert !== "string" || !rule.alert) { + errors.push(`${path}: a rule in group "${group.name}" is missing "alert"`); + } + if (typeof rule.expr !== "string" || !rule.expr) { + errors.push(`${path}: rule "${label}" is missing a non-empty "expr"`); + } else { + const issue = promqlSanityIssue(rule.expr); + if (issue) errors.push(`${path}: rule "${label}" has a suspect "expr" — ${issue}`); + } + if (typeof rule.labels?.severity !== "string" || !rule.labels.severity) { + errors.push(`${path}: rule "${label}" is missing "labels.severity"`); + } + if (typeof rule.annotations?.summary !== "string" || !rule.annotations.summary) { + errors.push(`${path}: rule "${label}" is missing "annotations.summary"`); + } + } + } + return errors; +} + +/* v8 ignore start -- CLI entrypoint; the exported functions above carry the tested logic. */ +function main() { + const errors = [...validateDashboards("grafana/dashboards"), ...validateAlertRules("prometheus/rules/alerts.yml")]; + if (errors.length > 0) { + console.error(`validate-observability-configs: ${errors.length} problem(s) found:`); + for (const e of errors) console.error(` - ${e}`); + process.exit(1); + } + console.log("validate-observability-configs: dashboards and alert rules are valid"); +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) main(); +/* v8 ignore stop */ diff --git a/test/unit/validate-observability-configs-script.test.ts b/test/unit/validate-observability-configs-script.test.ts new file mode 100644 index 0000000000..9fa5410a50 --- /dev/null +++ b/test/unit/validate-observability-configs-script.test.ts @@ -0,0 +1,188 @@ +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { validateAlertRules, validateDashboards } from "../../scripts/validate-observability-configs.mjs"; + +function tmpDashboardDir(files: Record): string { + const dir = mkdtempSync(join(tmpdir(), "gt-dash-")); + for (const [name, content] of Object.entries(files)) { + writeFileSync(join(dir, name), content); + } + return dir; +} + +function tmpAlertFile(content: string): string { + const dir = mkdtempSync(join(tmpdir(), "gt-alerts-")); + const path = join(dir, "alerts.yml"); + writeFileSync(path, content); + return path; +} + +describe("validate-observability-configs (#1943)", () => { + it("passes the real dashboards and alert rules shipped in the repo", () => { + expect(validateDashboards("grafana/dashboards")).toEqual([]); + expect(validateAlertRules("prometheus/rules/alerts.yml")).toEqual([]); + }); + + describe("validateDashboards", () => { + it("flags invalid JSON", () => { + const dir = tmpDashboardDir({ "broken.json": "{ not json" }); + const errors = validateDashboards(dir); + expect(errors.some((e) => e.includes("invalid JSON"))).toBe(true); + }); + + it("flags a dashboard missing title or panels", () => { + const dir = tmpDashboardDir({ "incomplete.json": JSON.stringify({}) }); + const errors = validateDashboards(dir); + expect(errors.some((e) => e.includes('missing a non-empty top-level "title"'))).toBe(true); + expect(errors.some((e) => e.includes('missing a top-level "panels" array'))).toBe(true); + }); + + it("flags an empty dashboards directory", () => { + const dir = mkdtempSync(join(tmpdir(), "gt-dash-empty-")); + const errors = validateDashboards(dir); + expect(errors).toEqual([`${dir}: no dashboard JSON files found`]); + }); + + it("passes a well-formed dashboard", () => { + const dir = tmpDashboardDir({ + "ok.json": JSON.stringify({ title: "OK Dashboard", panels: [] }), + }); + expect(validateDashboards(dir)).toEqual([]); + }); + + it("reports the directory itself when unreadable", () => { + const errors = validateDashboards(join(tmpdir(), "gt-does-not-exist-" + Math.random())); + expect(errors.length).toBe(1); + expect(errors[0]).toContain("could not read directory"); + }); + + it("reports a validation error (not a crash) when a dashboard file is valid JSON but not an object", () => { + const dir = tmpDashboardDir({ "null.json": "null", "array.json": "[]", "string.json": '"hi"' }); + const errors = validateDashboards(dir); + expect(errors.some((e) => e.includes("null.json") && e.includes("must be a JSON object"))).toBe(true); + expect(errors.some((e) => e.includes("array.json") && e.includes("must be a JSON object"))).toBe(true); + expect(errors.some((e) => e.includes("string.json") && e.includes("must be a JSON object"))).toBe(true); + }); + }); + + describe("validateAlertRules", () => { + it("flags invalid YAML", () => { + const path = tmpAlertFile("groups:\n - name: [unterminated"); + const errors = validateAlertRules(path); + expect(errors.some((e) => e.includes("invalid YAML"))).toBe(true); + }); + + it("flags a missing top-level groups array", () => { + const path = tmpAlertFile("not_groups: []"); + expect(validateAlertRules(path)).toEqual([`${path}: missing a top-level "groups" array`]); + }); + + it("flags a group missing name or rules", () => { + const path = tmpAlertFile("groups:\n - {}\n"); + const errors = validateAlertRules(path); + expect(errors.some((e) => e.includes('missing a non-empty "name"'))).toBe(true); + expect(errors.some((e) => e.includes('missing a "rules" array'))).toBe(true); + }); + + it("flags a rule missing alert, expr, severity, or summary", () => { + const path = tmpAlertFile( + ["groups:", " - name: test-group", " rules:", " - expr: up == 0"].join("\n"), + ); + const errors = validateAlertRules(path); + expect(errors.some((e) => e.includes('missing "alert"'))).toBe(true); + expect(errors.some((e) => e.includes('missing "labels.severity"'))).toBe(true); + expect(errors.some((e) => e.includes('missing "annotations.summary"'))).toBe(true); + }); + + it("passes a well-formed rule", () => { + const path = tmpAlertFile( + [ + "groups:", + " - name: test-group", + " rules:", + " - alert: TestAlert", + " expr: up == 0", + " for: 5m", + " labels:", + " severity: warning", + " annotations:", + " summary: test", + ].join("\n"), + ); + expect(validateAlertRules(path)).toEqual([]); + }); + + it("reports the file itself when unreadable", () => { + const path = join(tmpdir(), "gt-does-not-exist-" + Math.random() + ".yml"); + const errors = validateAlertRules(path); + expect(errors.length).toBe(1); + expect(errors[0]).toContain("invalid YAML"); + }); + + it("reports a validation error (not a crash) when a group entry is not an object", () => { + const path = tmpAlertFile("groups:\n - null\n - 5\n - just-a-string\n"); + const errors = validateAlertRules(path); + expect(errors.length).toBe(3); + for (const e of errors) expect(e).toContain("must be an object"); + }); + + it("reports a validation error (not a crash) when a rule entry is not an object", () => { + const path = tmpAlertFile(["groups:", " - name: test-group", " rules:", " - null"].join("\n")); + const errors = validateAlertRules(path); + expect(errors.some((e) => e.includes("rules[0]") && e.includes("must be an object"))).toBe(true); + }); + + function ruleFile(expr: string): string { + return tmpAlertFile( + [ + "groups:", + " - name: test-group", + " rules:", + " - alert: TestAlert", + ` expr: ${expr}`, + " labels:", + " severity: warning", + " annotations:", + " summary: test", + ].join("\n"), + ); + } + + it("flags an expr with a dangling binary operator", () => { + const errors = validateAlertRules(ruleFile("up ==")); + expect(errors.some((e) => e.includes("dangling") || e.includes('binary operator "=="'))).toBe(true); + }); + + it("flags an expr with unbalanced brackets", () => { + const errors = validateAlertRules(ruleFile("sum(rate(foo[5m])")); + expect(errors.some((e) => e.includes("unbalanced brackets"))).toBe(true); + }); + + it("flags an expr with an unexpected closing bracket", () => { + const errors = validateAlertRules(ruleFile("up)")); + expect(errors.some((e) => e.includes("unbalanced brackets"))).toBe(true); + }); + + it("flags mismatched bracket TYPES even when nesting depth balances out (REGRESSION)", () => { + // sum(foo[5m)) -- opened with "(" and "[", closed with ")" and ")". A naive depth counter (net + // opens - closes = 0) wrongly calls this balanced; the closer must match its actual opener. + const errors = validateAlertRules(ruleFile("sum(foo[5m))")); + expect(errors.some((e) => e.includes("unbalanced brackets") && e.includes('expected the match for "["'))).toBe( + true, + ); + }); + + it("passes well-formed real-shaped PromQL expressions", () => { + for (const expr of [ + "up == 0", + "sum(rate(gittensory_jobs_total[5m])) by (status) > 10", + 'histogram_quantile(0.95, rate(gittensory_http_duration_seconds_bucket{route="/health"}[5m]))', + "(a + b) / c", + ]) { + expect(validateAlertRules(ruleFile(expr))).toEqual([]); + } + }); + }); +});