Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions docs/slo.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions scripts/__tests__/validate-schema-cli-discover.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions scripts/validate-schema.mjs
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
Loading