diff --git a/docs/slo.md b/docs/slo.md index 38f83b740..5d7210d33 100644 --- a/docs/slo.md +++ b/docs/slo.md @@ -51,15 +51,18 @@ No exporter or external data flow is added by this document — recommendations as a follow-up (see below); recovery guidance is in the same [`runbooks/incident-response-unsafe-mission-merge.md`](../runbooks/incident-response-unsafe-mission-merge.md). **Third known exception**: `Validate Mission Schema` itself has a broader - version of this same gap — it never validates `runbooks/**` at all, on - *either* trigger. Its PR-mode `git diff` pathspec covers only - `fixes/**/*.json`/`*.yaml`/`*.yml`, so a `runbooks/**`-only PR resolves to - an empty file list and the validation step is skipped (job still reports - green). And its scheduled/push `--all` mode - (`scripts/validate-schema.mjs`) only calls `discoverMissionFiles('fixes')` - — the weekly cadence sweep never walks `runbooks/**` either. The 10 - `runbooks/*.json` mission files therefore have zero automated schema - validation coverage on any trigger. Also tracked as a follow-up (see + version of this same gap — it never validated `runbooks/**` at all, on + *either* trigger. Its PR-mode `git diff` pathspec covered only + `fixes/**/*.json`/`*.yaml`/`*.yml`, so a `runbooks/**`-only PR resolved to + an empty file list and the validation step was skipped (job still reported + green). The scheduled/push `--all` mode side of this gap + (`scripts/validate-schema.mjs` only walking `fixes/`) has been fixed — it + now also discovers mission files under `runbooks/`, so the weekly cadence + sweep validates all 10 `runbooks/*.json` files. The PR-mode pathspec still + needs the corresponding `runbooks/**/*.json`/`*.yaml`/`*.yml` globs added + to `.github/workflows/validate-schema.yml`'s "Find changed files (PR + only)" step; that edit is prepared but requires `workflows` permission + this contribution's credentials do not have. Tracked as a follow-up (see below); recovery guidance is in the same [`runbooks/incident-response-unsafe-mission-merge.md`](../runbooks/incident-response-unsafe-mission-merge.md). **Fourth known exception**: `KB Quality Enforcement` @@ -143,14 +146,19 @@ dangerous commands" step. Also filed separately as a `[operations]` issue for the same `workflows`-permission reason. The section 2 "third known exception" above (`validate-schema.yml` never -validating `runbooks/**`, on PRs or on its scheduled/push `--all` sweep) -also requires editing that workflow's PR-mode `git diff` pathspec and -`scripts/validate-schema.mjs`'s `--all` branch to also discover files under -`runbooks/`. Also filed separately as a `[operations]` issue (#3255) for the -same `workflows`-permission reason. +validating `runbooks/**`) is now partially resolved: +`scripts/validate-schema.mjs`'s `--all` branch has been updated to also +discover files under `runbooks/`, so the weekly/push sweep now covers all +10 `runbooks/*.json` files. The remaining piece — extending +`validate-schema.yml`'s PR-mode `git diff` pathspec with the same +`runbooks/**/*.json`/`*.yaml`/`*.yml` globs so a `runbooks/**`-only PR is +no longer skipped — still requires `workflows` permission this +contribution's credentials do not have. Tracked in `[operations]` issue +#3255 until that pathspec change lands. ## References + - [`runbooks/incident-response-index-publish-failure.md`](../runbooks/incident-response-index-publish-failure.md) - [`runbooks/incident-response-search-state-corruption.md`](../runbooks/incident-response-search-state-corruption.md) — covers the `CNCF Mission Generation` workflow's separate direct-to-`master` push of `search-state.json`, which (unlike `fixes/index.json`) has no content-validation gate at all - [`runbooks/incident-response-unsafe-mission-merge.md`](../runbooks/incident-response-unsafe-mission-merge.md) — covers the `CNCF Mission Generation` workflow's `--admin` auto-merge bypassing `Mission Safety Scan` and `Validate Mission Schema`, and separately, `Mission Safety Scan`'s own false-green on `runbooks/**`-only PRs diff --git a/scripts/__tests__/validate-schema-cli-discover.test.mjs b/scripts/__tests__/validate-schema-cli-discover.test.mjs index 5604244d3..bdbeb6637 100644 --- a/scripts/__tests__/validate-schema-cli-discover.test.mjs +++ b/scripts/__tests__/validate-schema-cli-discover.test.mjs @@ -191,6 +191,49 @@ describe('validate-schema.mjs --all discovery', () => { }) }) + it('also discovers mission files under runbooks/, in addition to fixes/', () => { + // Guards against the false-green regression where --all only ever + // scanned fixes/: runbooks/*.json uses the same kc-mission-v1 schema + // (see runbooks/README.md) but previously had zero scheduled/push + // validation coverage because ALL_MODE_DIRS omitted it. + withTempDir(dir => { + mkdirSync(join(dir, 'fixes'), { recursive: true }) + mkdirSync(join(dir, 'runbooks'), { recursive: true }) + writeFileSync(join(dir, 'fixes', 'a.json'), JSON.stringify(VALID_MISSION)) + writeFileSync(join(dir, 'runbooks', 'b.json'), JSON.stringify(VALID_MISSION)) + + const result = runCli(dir, ['--all']) + + expect(result.status).toBe(0) + expect(result.stdout).toMatch(/Discovered 2 mission files to validate\./) + expect(result.stdout).toContain('a.json') + expect(result.stdout).toContain('b.json') + + const summary = parseSummary(result.stdout) + expect(summary).toMatchObject({ + trigger: 'all', + total: 2, + validCount: 2, + invalidCount: 0, + }) + }) + }) + + it('--all mode still works when runbooks/ does not exist (no crash)', () => { + // ALL_MODE_DIRS must tolerate a missing runbooks/ directory rather than + // throwing ENOENT, so this doesn't regress environments/checkouts that + // don't have one. + withTempDir(dir => { + mkdirSync(join(dir, 'fixes'), { recursive: true }) + writeFileSync(join(dir, 'fixes', 'a.json'), JSON.stringify(VALID_MISSION)) + + const result = runCli(dir, ['--all']) + + expect(result.status).toBe(0) + expect(result.stdout).toMatch(/Discovered 1 mission files to validate\./) + }) + }) + it('sets summary trigger to "all" and level to "error" when a discovered file is invalid', () => { // Guards the trigger vs level distinction: trigger reflects the CLI // switch, level reflects the outcome. A regression that swapped them diff --git a/scripts/validate-schema.mjs b/scripts/validate-schema.mjs index dc850866f..8a3ccdc02 100644 --- a/scripts/validate-schema.mjs +++ b/scripts/validate-schema.mjs @@ -1,5 +1,5 @@ #!/usr/bin/env node -import { readFileSync, readdirSync } from 'fs'; +import { readFileSync, readdirSync, existsSync } from 'fs'; import { join } from 'path'; import * as yaml from 'js-yaml'; import { validateMissionExport } from './scanner.mjs'; @@ -13,6 +13,14 @@ const MISSION_EXTENSIONS = new Set(['.json', '.yaml', '.yml']); /** Files to skip when discovering all missions */ const SKIP_FILENAMES = new Set(['index.json']); +/** + * Directories scanned in `--all` mode. `runbooks/` holds the same + * `kc-mission-v1` schema format as `fixes/` (see runbooks/README.md) but was + * previously omitted here, leaving its mission files with no scheduled/push + * schema-validation coverage. + */ +const ALL_MODE_DIRS = ['fixes', 'runbooks']; + /** * Recursively discovers all mission files under the given directory. * Returns an array of relative file paths. @@ -99,8 +107,11 @@ function main() { let files; if (args.includes('--all')) { - // Discover all mission files under fixes/ (used for push/schedule/dispatch) - files = discoverMissionFiles('fixes'); + // Discover all mission files under fixes/ and runbooks/ (used for + // push/schedule/dispatch full sweeps). + files = ALL_MODE_DIRS + .filter(dir => existsSync(dir)) + .flatMap(dir => discoverMissionFiles(dir)); console.log(`Discovered ${files.length} mission files to validate.\n`); } else { files = args.flatMap(a => a.split(/\s+/)).filter(Boolean);