Skip to content

Commit ff0aa26

Browse files
gsoldevilacursoragentkibanamachine
authored
[SavedObjects] Improve schema-only mutation handling: serialized schemas + granular diff (#268630)
## Summary Extends the schema-only mutation tolerance introduced in #256432 with the following improvements: ### 1. Serialised schemas in snapshots Instead of a 64-byte SHA-256 hash, each model version's \`create\` and \`forwardCompatibility\` schemas are now stored as their full Joi \`describe()\` output (\`Record<string, unknown>\`). This enables field-level diffing across CI runs. > **Backward compatibility** — when the current snapshot contains a Joi object but the GCS baseline still contains the old hash string, the check computes a legacy hash on-the-fly (via \`computeSchemaHash\`) and compares it against the stored hash. This keeps cross-format comparisons correct during the transition window, including for function-based \`forwardCompatibility\` schemas (e.g. fleet, SLO), which the old code hashed as \`SHA256(fn.toString())\`. ### 2. Granular schema diff When the CI check detects a schema-only mutation, it now classifies each field change: | Change | \`create\` schema | \`forwardCompatibility\` schema | |--------|-----------------|-------------------------------| | Schema removed from model version | ❌ error | ❌ error | | Field removed | ❌ error | ❌ error | | Field type changed | ❌ error | ❌ error | | Optional → required / new required field | ❌ error | ❌ error | | New optional field added | ✅ allow | ✅ allow | | Required → optional field | ✅ allow | ✅ allow | | Constraint tightened / validator added | ⚠️ warning | ⚠️ warning | | Constraint loosened / validator removed | ✅ allow | ✅ allow | When the diff cannot determine whether a change is safe (baseline uses a legacy hash string), CI **throws** instead of warning, forcing a baseline regeneration before the PR can merge. ### 3. Complete migration to \`SavedObjectsCheckError\` All remaining \`throw new Error()\` calls in the validator have been migrated to \`SavedObjectsCheckError\` (introduced by #268469). Each violation now carries a structured \`ruleId\`, \`fixHint\`, and \`docsAnchor\` for actionable CI PR comments. New \`RULE_IDS\` added: | Rule ID | Violation | |---------|-----------| | \`existing-type/schema-breaking-changes\` | Breaking schema change in an existing model version | | \`existing-type/schema-undiffable-legacy-hash\` | Schema changed but baseline uses legacy hash — cannot diff | | \`existing-type/new-mappings-not-in-model-version\` | New mapping fields not declared in the model version | | \`existing-type/keyword-missing-ignore-above\` | Newly introduced keyword/flattened fields without \`ignore_above\` | | \`existing-type/invalid-name-title-field-type\` | New name/title field with incorrect mapping type | | \`model-version/mappings-not-in-schema\` | Mapping fields absent from the latest model version schema | | \`model-version/mapping-index-false\` | New mapping field uses \`index: false\` | | \`model-version/mapping-enabled-false\` | New mapping field uses \`enabled: false\` | ### Snapshot size | Format | On-disk size | |--------|-------------| | Old (hash strings) | ~137 KB | | Full Joi \`describe()\` output, pretty-printed (rejected) | ~15 MB | | Full Joi \`describe()\` output, compact JSON (current) | **~1.7 MB** | Snapshots are stored in GCS and not committed to the repository. Mock JSON files under \`src/snapshots/mocks/\` remain pretty-printed for readability. ## Test plan - [x] All unit tests in \`kbn-check-saved-objects-cli\` and \`so-type-serializer\` pass - [x] Tests cover: field removal (throws), schema removal (throws), required field addition (throws), optional field addition (allows), constraint tightening (warns) - [x] Tests for backward compat against hash-based baselines: schema unchanged (no false positive), function-based schema unchanged (no false positive), schema changed against hash baseline (throws) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
1 parent 4ddbb93 commit ff0aa26

15 files changed

Lines changed: 1294 additions & 4385 deletions

packages/kbn-check-saved-objects-cli/src/commands/run_so_migration_snapshot_cli.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export const runSoMigrationSnapshotCli = () => {
4343
const snapshot = await takeSnapshot(types);
4444

4545
await mkdir(dirname(outputPath), { recursive: true });
46-
await writeFile(outputPath, JSON.stringify(snapshot, null, 2), { encoding: 'utf-8' });
46+
await writeFile(outputPath, JSON.stringify(snapshot), { encoding: 'utf-8' });
4747

4848
log.info('Snapshot successfully taken to: ' + outputPath);
4949
},

packages/kbn-check-saved-objects-cli/src/findings/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ export const RULE_IDS = {
1515
EXISTING_TYPE_MAPPINGS_WITHOUT_NEW_MODEL_VERSION:
1616
'existing-type/mappings-changed-without-new-model-version',
1717
EXISTING_TYPE_MUTATED_MODEL_VERSION: 'existing-type/mutated-existing-model-version',
18+
EXISTING_TYPE_SCHEMA_BREAKING_CHANGES: 'existing-type/schema-breaking-changes',
19+
EXISTING_TYPE_SCHEMA_UNDIFFABLE: 'existing-type/schema-undiffable-legacy-hash',
20+
EXISTING_TYPE_NEW_MAPPINGS_NOT_IN_MODEL_VERSION:
21+
'existing-type/new-mappings-not-in-model-version',
22+
EXISTING_TYPE_KEYWORD_MISSING_IGNORE_ABOVE: 'existing-type/keyword-missing-ignore-above',
23+
EXISTING_TYPE_INVALID_NAME_TITLE_FIELD_TYPE: 'existing-type/invalid-name-title-field-type',
1824
// new types
1925
NEW_TYPE_LEGACY_MIGRATIONS: 'new-type/legacy-migrations',
2026
NEW_TYPE_MISSING_INITIAL_MODEL_VERSION: 'new-type/missing-initial-model-version',
@@ -24,6 +30,9 @@ export const RULE_IDS = {
2430
MODEL_VERSION_MISSING_SCHEMAS: 'model-version/missing-schemas',
2531
MODEL_VERSION_MISSING_FORWARD_COMPATIBILITY: 'model-version/missing-forward-compatibility',
2632
MODEL_VERSION_MISSING_CREATE_SCHEMA: 'model-version/missing-create-schema',
33+
MODEL_VERSION_MAPPINGS_NOT_IN_SCHEMA: 'model-version/mappings-not-in-schema',
34+
MODEL_VERSION_MAPPING_INDEX_FALSE: 'model-version/mapping-index-false',
35+
MODEL_VERSION_MAPPING_ENABLED_FALSE: 'model-version/mapping-enabled-false',
2736
// removed types
2837
REMOVED_TYPE_NAME_REUSED: 'removed-type/name-reused',
2938
REMOVED_TYPE_NEEDS_UPDATE: 'removed-type/registry-needs-update',

0 commit comments

Comments
 (0)