Skip to content

Commit 9710fb8

Browse files
kibanamachinedelanniCopilot
authored
[9.1] [ci] Lint yamls upon commit (#235181) (#235330)
# Backport This will backport the following commits from `main` to `9.1`: - [[ci] Lint yamls upon commit (#235181)](#235181) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Alex Szabo","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-09-17T08:56:58Z","message":"[ci] Lint yamls upon commit (#235181)\n\n## Summary\nWe've seen earlier where we've edited yaml files, and managed to push\nthem with some syntax issues. This adds a safeguard locally to try to\navoid those issues.\n\n- Refactors `run_precommit_hook.js` to allow for more checks\n- Adds a yaml linting step to the pre-commit hook\n- (unrelated) Clarifies missing bootstrap error (*)\n\n\n(*)\nIf you've wanted to get the TS project list after a branch switch, it\ngave this error:\n```\nts_project.ts:48\n matches: knownPaths.filter(matcher),\n ^\nTypeError: knownPaths.filter is not a function\n at map (ts_project.ts:48:25)\n```\nMeaning, you should re-bootstrap. It's now more explicit with an error:\n```\nts_project.ts:76\n throw new Error('TS Project map missing, make sure you run `yarn kbn bootstrap`');\n ^\nError: TS Project map missing, make sure you run `yarn kbn bootstrap`\n at Function.loadAll (ts_project.ts:76:13)\n```\n\n---------\n\nCo-authored-by: Copilot <[email protected]>","sha":"7bca91c3e415608574ad94d62f0c78b6ef136003","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Operations","release_note:skip","backport:all-open","v9.2.0"],"title":"[ci] Lint yamls upon commit","number":235181,"url":"https://github.com/elastic/kibana/pull/235181","mergeCommit":{"message":"[ci] Lint yamls upon commit (#235181)\n\n## Summary\nWe've seen earlier where we've edited yaml files, and managed to push\nthem with some syntax issues. This adds a safeguard locally to try to\navoid those issues.\n\n- Refactors `run_precommit_hook.js` to allow for more checks\n- Adds a yaml linting step to the pre-commit hook\n- (unrelated) Clarifies missing bootstrap error (*)\n\n\n(*)\nIf you've wanted to get the TS project list after a branch switch, it\ngave this error:\n```\nts_project.ts:48\n matches: knownPaths.filter(matcher),\n ^\nTypeError: knownPaths.filter is not a function\n at map (ts_project.ts:48:25)\n```\nMeaning, you should re-bootstrap. It's now more explicit with an error:\n```\nts_project.ts:76\n throw new Error('TS Project map missing, make sure you run `yarn kbn bootstrap`');\n ^\nError: TS Project map missing, make sure you run `yarn kbn bootstrap`\n at Function.loadAll (ts_project.ts:76:13)\n```\n\n---------\n\nCo-authored-by: Copilot <[email protected]>","sha":"7bca91c3e415608574ad94d62f0c78b6ef136003"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/235181","number":235181,"mergeCommit":{"message":"[ci] Lint yamls upon commit (#235181)\n\n## Summary\nWe've seen earlier where we've edited yaml files, and managed to push\nthem with some syntax issues. This adds a safeguard locally to try to\navoid those issues.\n\n- Refactors `run_precommit_hook.js` to allow for more checks\n- Adds a yaml linting step to the pre-commit hook\n- (unrelated) Clarifies missing bootstrap error (*)\n\n\n(*)\nIf you've wanted to get the TS project list after a branch switch, it\ngave this error:\n```\nts_project.ts:48\n matches: knownPaths.filter(matcher),\n ^\nTypeError: knownPaths.filter is not a function\n at map (ts_project.ts:48:25)\n```\nMeaning, you should re-bootstrap. It's now more explicit with an error:\n```\nts_project.ts:76\n throw new Error('TS Project map missing, make sure you run `yarn kbn bootstrap`');\n ^\nError: TS Project map missing, make sure you run `yarn kbn bootstrap`\n at Function.loadAll (ts_project.ts:76:13)\n```\n\n---------\n\nCo-authored-by: Copilot <[email protected]>","sha":"7bca91c3e415608574ad94d62f0c78b6ef136003"}}]}] BACKPORT--> Co-authored-by: Alex Szabo <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 20ce0c0 commit 9710fb8

File tree

3 files changed

+155
-28
lines changed

3 files changed

+155
-28
lines changed

‎packages/kbn-ts-projects/ts_project.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ export class TsProject {
7171

7272
const tsConfigRepoRels: string[] = JSON.parse(Fs.readFileSync(mapPath, 'utf8'));
7373

74+
if (!tsConfigRepoRels || !tsConfigRepoRels.length) {
75+
throw new Error('TS Project map missing, make sure you run `yarn kbn bootstrap`');
76+
}
77+
7478
const ignores = expand('ignore', options.ignore, tsConfigRepoRels);
7579
const disableTypeCheck = expand('disableTypeCheck', options.disableTypeCheck, tsConfigRepoRels);
7680

‎src/dev/run_precommit_hook.js‎

Lines changed: 150 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,141 @@
1010
import SimpleGit from 'simple-git';
1111

1212
import { run } from '@kbn/dev-cli-runner';
13-
import { createFlagError, combineErrors } from '@kbn/dev-cli-errors';
13+
import { createFlagError } from '@kbn/dev-cli-errors';
1414
import { REPO_ROOT } from '@kbn/repo-info';
1515
import * as Eslint from './eslint';
1616
import * as Stylelint from './stylelint';
1717
import { getFilesForCommit, checkFileCasing } from './precommit_hook';
18+
import { load as yamlLoad } from 'js-yaml';
19+
import { readFile } from 'fs/promises';
20+
import { extname } from 'path';
21+
22+
class CheckResult {
23+
constructor(checkName) {
24+
this.checkName = checkName;
25+
this.errors = [];
26+
this.succeeded = true;
27+
}
28+
29+
addError(error) {
30+
this.succeeded = false;
31+
this.errors.push(error);
32+
}
33+
34+
toString() {
35+
if (this.succeeded) {
36+
return `✓ ${this.checkName}: Passed`;
37+
} else {
38+
return [`✗ ${this.checkName}: Failed`, ...this.errors.map((err) => ` - ${err}`)].join('\n');
39+
}
40+
}
41+
}
42+
43+
class PrecommitCheck {
44+
constructor(name) {
45+
this.name = name;
46+
}
47+
48+
async execute() {
49+
throw new Error('execute() must be implemented by check class');
50+
}
51+
52+
async runSafely(log, files, options) {
53+
const result = new CheckResult(this.name);
54+
try {
55+
await this.execute(log, files, options);
56+
} catch (error) {
57+
if (error.errors) {
58+
error.errors.forEach((err) => result.addError(err.message || err.toString()));
59+
} else {
60+
result.addError(error.message || error.toString());
61+
}
62+
}
63+
return result;
64+
}
65+
}
66+
67+
class FileCasingCheck extends PrecommitCheck {
68+
constructor() {
69+
super('File Casing');
70+
}
71+
72+
async execute(log, files) {
73+
await checkFileCasing(log, files);
74+
}
75+
}
76+
77+
class LinterCheck extends PrecommitCheck {
78+
constructor(name, linter) {
79+
super(name);
80+
this.linter = linter;
81+
}
82+
83+
async execute(log, files, options) {
84+
const filesToLint = await this.linter.pickFilesToLint(log, files);
85+
if (filesToLint.length > 0) {
86+
await this.linter.lintFiles(log, filesToLint, {
87+
fix: options.fix,
88+
});
89+
90+
if (options.fix && options.stage) {
91+
const simpleGit = new SimpleGit(REPO_ROOT);
92+
await simpleGit.add(filesToLint);
93+
}
94+
}
95+
}
96+
}
97+
98+
class YamlLintCheck extends PrecommitCheck {
99+
constructor() {
100+
super('YAML Lint');
101+
}
102+
103+
isYamlFile(filePath) {
104+
const ext = extname(filePath).toLowerCase();
105+
return ext === '.yml' || ext === '.yaml';
106+
}
107+
108+
async execute(log, files) {
109+
const yamlFiles = files.filter((file) => this.isYamlFile(file.getRelativePath()));
110+
111+
if (yamlFiles.length === 0) {
112+
log.verbose('No YAML files to check');
113+
return;
114+
}
115+
116+
log.verbose(`Checking ${yamlFiles.length} YAML files for syntax errors`);
117+
118+
const errors = [];
119+
for (const file of yamlFiles) {
120+
try {
121+
const content = await readFile(file.getAbsolutePath(), 'utf8');
122+
yamlLoad(content, {
123+
filename: file.getRelativePath(),
124+
});
125+
} catch (error) {
126+
errors.push(`Error in ${file.getRelativePath()}:\n${error.message}`);
127+
}
128+
}
129+
130+
if (errors.length > 0) {
131+
throw new Error(errors.join('\n\n'));
132+
}
133+
}
134+
}
135+
136+
const PRECOMMIT_CHECKS = [
137+
new FileCasingCheck(),
138+
new LinterCheck('ESLint', Eslint),
139+
new LinterCheck('StyleLint', Stylelint),
140+
new YamlLintCheck(),
141+
];
18142

19143
run(
20144
async ({ log, flags }) => {
21145
process.env.IS_KIBANA_PRECOMIT_HOOK = 'true';
22146

23147
const files = await getFilesForCommit(flags.ref);
24-
const errors = [];
25148

26149
const maxFilesCount = flags['max-files']
27150
? Number.parseInt(String(flags['max-files']), 10)
@@ -37,33 +160,33 @@ run(
37160
return;
38161
}
39162

40-
try {
41-
await checkFileCasing(log, files);
42-
} catch (error) {
43-
errors.push(error);
44-
}
163+
log.verbose('Running pre-commit checks...');
164+
const results = await Promise.all(
165+
PRECOMMIT_CHECKS.map(async (check) => {
166+
const startTime = Date.now();
167+
const result = await check.runSafely(log, files, {
168+
fix: flags.fix,
169+
stage: flags.stage,
170+
});
171+
const duration = Date.now() - startTime;
172+
log.verbose(`${check.name} completed in ${duration}ms`);
173+
return result;
174+
})
175+
);
45176

46-
for (const Linter of [Eslint, Stylelint]) {
47-
const filesToLint = await Linter.pickFilesToLint(log, files);
48-
if (filesToLint.length > 0) {
49-
try {
50-
await Linter.lintFiles(log, filesToLint, {
51-
fix: flags.fix,
52-
});
53-
54-
if (flags.fix && flags.stage) {
55-
const simpleGit = new SimpleGit(REPO_ROOT);
56-
await simpleGit.add(filesToLint);
57-
}
58-
} catch (error) {
59-
errors.push(error);
60-
}
61-
}
62-
}
177+
const failedChecks = results.filter((result) => !result.succeeded);
178+
179+
if (failedChecks.length > 0) {
180+
const errorReport = [
181+
'\nPre-commit checks failed:',
182+
...results.map((result) => result.toString()),
183+
'\nPlease fix the above issues before committing.',
184+
].join('\n');
63185

64-
if (errors.length) {
65-
throw combineErrors(errors);
186+
throw new Error(errorReport);
66187
}
188+
189+
log.success('All pre-commit checks passed!');
67190
},
68191
{
69192
description: `
@@ -77,7 +200,7 @@ run(
77200
stage: true,
78201
},
79202
help: `
80-
--fix Execute eslint in --fix mode
203+
--fix Execute checks with possible fixes
81204
--max-files Max files number to check against. If exceeded the script will skip the execution
82205
--ref Run checks against any git ref files (example HEAD or <commit_sha>) instead of running against staged ones
83206
--no-stage By default when using --fix the changes are staged, use --no-stage to disable that behavior

‎src/platform/packages/shared/kbn-dev-utils/src/precommit_hook/script_source.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ execute_precommit_hook() {
9999
100100
execute_precommit_hook || {
101101
echo "Pre-commit hook failed (add --no-verify to bypass)";
102-
echo ' For eslint failures you can try running \`node scripts/precommit_hook --fix\`';
102+
echo ' For fixable failures you can try running \`node scripts/precommit_hook --fix\`';
103103
exit 1;
104104
}
105105

0 commit comments

Comments
 (0)