From 6101a1af57515a0e6ccfb57de532e09f2e06638c Mon Sep 17 00:00:00 2001 From: mbwhite Date: Fri, 1 Mar 2024 11:36:19 +0000 Subject: [PATCH] feat: custom rules Signed-off-by: mbwhite --- .tektonlintrc.yaml | 5 +- README.md | 46 ++++++++++++++++++ custom_rules/README.md | 47 +++++++++++++++++++ custom_rules/my_rules.js | 25 ++++++++++ custom_rules/package.json | 12 +++++ package-lock.json | 7 +++ .../general/no-invalid-name.yaml.expect.json | 1 + src/config.ts | 6 +++ src/interfaces/common.ts | 3 ++ src/rule-loader.ts | 28 ++++++++++- src/rules.ts | 8 ++-- src/runner.ts | 2 +- 12 files changed, 183 insertions(+), 7 deletions(-) create mode 100644 custom_rules/README.md create mode 100644 custom_rules/my_rules.js create mode 100644 custom_rules/package.json create mode 100644 regression-tests/general/no-invalid-name.yaml.expect.json diff --git a/.tektonlintrc.yaml b/.tektonlintrc.yaml index 001d5d1..087a332 100644 --- a/.tektonlintrc.yaml +++ b/.tektonlintrc.yaml @@ -23,4 +23,7 @@ rules: # error | warning | off prefer-when-expression: warning no-deprecated-resource: warning no-missing-hashbang: warning - + +# custom: +# my_rules: custom_rules + diff --git a/README.md b/README.md index e0b8cdc..026e6ef 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ A linter for Tekton resource definitions - **v1 beta now available!!** - [Detecting errors](#detecting-errors) - [Best practices](#best-practices) - [Configuring tekton-lint](#configuring-tekton-lint) + - [Creating custom rules](#custom-rules) - [Issues?](#issues) @@ -228,6 +229,51 @@ Search path for `.tektonlintrc.yaml` - current working directory - default values used if nothing else found +## Custom Rules +In additional the default set of rules, custom rules can provided as node modules; the mechanism is similar that used by tools such as eslint. + +An example is provided in [`custom_rules`](./custom_rules), and will be used in the examples below. Notes on writing rules are also in the [README.md](./custom_rules/README.md) + +### Configuring + +In the `.tektonrc.yaml` file add an object with names of the rules and node module that provides it. +To load rules exported by the module `custom_rules`, and named under `my_rules` add the `custom` field to the yaml file. + +```yaml +--- +rules: + .... +external:tasks: + ... +custom: + my_rules: custom_rules + # For debug and test, refer directly to the js file + # my_rules: ../customer_rules/my_rules.js +``` + +### Reporting + +A module may report more than one rule. There is one rule in the example, and this flags up any task that starts with the work 'Task' (not meant to be serious rule, but an example!) + +Running on the `example-task.yaml` file with the example configured adds a 4th report + +``` +./example-task.yaml + 10:14 warning Invalid image: 'busybox'. Specify the image tag instead of using ':latest' no-latest-image + 9:31 error Undefined param 'contextDir' at .spec.steps[0].command[2] in 'Task-without-params' no-undefined-param + 11:19 error Undefined param 'contextDir' at .spec.steps[0].workingDir in 'Task-without-params' no-undefined-param + 1:1 error Tasks should not start with word 'Task' my_rules#no-tasks-called-task + +✖ 4 problems (3 errors, 1 warning) +``` + +Within the representation of the rule name here `my_rules#no-tasks-called-task`; if you want to turn off this rule, then you can adjust the list of rules in the `.tektonrc.yaml` as for another rule + +``` +rules: + ... + my_rules#no-tasks-called-task: off +``` ## Issues? diff --git a/custom_rules/README.md b/custom_rules/README.md new file mode 100644 index 0000000..67f6c06 --- /dev/null +++ b/custom_rules/README.md @@ -0,0 +1,47 @@ +# Tekton Lint custom rules + +- Create a regular node module +- Default export from this of an object +```js +module.exports = { + rules: [ + { + name: 'no-tasks-called-task', + reportDefault: 'error', + ruleFn: forbidTasksWithTaskInTheName + } + ] +} +``` + - Must contain a field called `rules` that is a array of objects + - Each 'rule' object, must hava name (kebab case preferred) + - Default report type (off/error/warning) + - Reference to the rules function + +- Rules are implemented in a function + +```js +// Example rule to check a name of a task, note this isn't meant +// to be a serious rule - rather as an example +const forbidTasksWithTaskInTheName = (docs, tekton, report) => { + + // try to always code defensively + if (tekton.tasks) { + for (const t of Object.values(tekton.tasks)) { + if (t.metadata.name.startsWith("Task")) { + report("Tasks should not start with word 'Task'", t) + } + } + } +}; +``` + + - `docs` are the full yaml docs + - `tekton` is the object with the parsed elements of the yaml files + - `report` is a fn to report rule breaks + + +## Development Notes: + +- As well as the yaml files from the project being tested, the rule will also see the cached elements +- try and code as defensively as possible, eg a set of files might not have Tasks diff --git a/custom_rules/my_rules.js b/custom_rules/my_rules.js new file mode 100644 index 0000000..1a89e59 --- /dev/null +++ b/custom_rules/my_rules.js @@ -0,0 +1,25 @@ + +// Example rule to check a name of a task, note this isn't meant +// to be a serious rule - rather as an example +const forbidTasksWithTaskInTheName = (docs, tekton, report) => { + + // try to always code defensively + if (tekton.tasks) { + for (const t of Object.values(tekton.tasks)) { + if (t.metadata.name.startsWith("Task")) { + report("Tasks should not start with word 'Task'", t) + } + } + } +}; + +// could export multiple rules if you wished here. +module.exports = { + rules: [ + { + name: 'no-tasks-called-task', + reportDefault: 'error', + ruleFn: forbidTasksWithTaskInTheName + } + ] +} diff --git a/custom_rules/package.json b/custom_rules/package.json new file mode 100644 index 0000000..00a0449 --- /dev/null +++ b/custom_rules/package.json @@ -0,0 +1,12 @@ +{ + "name": "custom_rules", + "version": "1.0.0", + "description": "Example of a custom rule", + "main": "my_rules.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": "", + "license": "ISC" +} diff --git a/package-lock.json b/package-lock.json index 6bae683..60e170c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,6 +11,7 @@ "dependencies": { "chalk": "^4.1.2", "chokidar": "^3.5.3", + "custom_rules": "file:custom_rules/custom_rules-1.0.0.tgz", "env-var": "^7.4.1", "fast-glob": "^3.3.2", "graphlib": "^2.1.8", @@ -3651,6 +3652,12 @@ "node": ">= 8" } }, + "node_modules/custom_rules": { + "version": "1.0.0", + "resolved": "file:custom_rules/custom_rules-1.0.0.tgz", + "integrity": "sha512-ydqZP3XE2ZQiNXn8eD8xvQxu3vvljtD81NnrWVR/mKZL0q1DIiltvD23bQOMyUTY4Oy1cPZ1bTs+l0rh6TNWYQ==", + "license": "ISC" + }, "node_modules/dargs": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/dargs/-/dargs-7.0.0.tgz", diff --git a/regression-tests/general/no-invalid-name.yaml.expect.json b/regression-tests/general/no-invalid-name.yaml.expect.json new file mode 100644 index 0000000..5047e57 --- /dev/null +++ b/regression-tests/general/no-invalid-name.yaml.expect.json @@ -0,0 +1 @@ +[{"message":"Task 'no-invalid-name' defines parameter '1-foo' with invalid parameter name (names are limited to alpha-numeric characters, '-' and '_' and can only start with alpha characters and '_')","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1767,1772,1782],"startLine":101,"startColumn":13,"endLine":101,"endColumn":18}},{"message":"Task 'no-invalid-name' defines parameter '-foo' with invalid parameter name (names are limited to alpha-numeric characters, '-' and '_' and can only start with alpha characters and '_')","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1794,1798,1808],"startLine":102,"startColumn":13,"endLine":102,"endColumn":17}},{"message":"Task 'no-invalid-name' defines parameter 'foo$bar' with invalid parameter name (names are limited to alpha-numeric characters, '-' and '_' and can only start with alpha characters and '_')","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1842,1849,1859],"startLine":104,"startColumn":13,"endLine":104,"endColumn":20}},{"message":"Invalid name for Task 'no-invalid-name-FooBar'. Names should be in lowercase, alphanumeric, kebab-case format. and follow DNS subdomain names","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[64,86,96],"startLine":5,"startColumn":9,"endLine":5,"endColumn":31}},{"message":"Invalid name for Task '-no-invalid-name'. Names should be in lowercase, alphanumeric, kebab-case format. and follow DNS subdomain names","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[178,194,204],"startLine":12,"startColumn":9,"endLine":12,"endColumn":25}},{"message":"Invalid name for Task '.no-invalid-name'. Names should be in lowercase, alphanumeric, kebab-case format. and follow DNS subdomain names","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[394,410,420],"startLine":26,"startColumn":9,"endLine":26,"endColumn":25}},{"message":"Invalid name for Task 'no-invalid-name_foo'. Names should be in lowercase, alphanumeric, kebab-case format. and follow DNS subdomain names","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[610,629,639],"startLine":40,"startColumn":9,"endLine":40,"endColumn":28}},{"message":"Invalid name for Task '1-no-invalid-name'. Names should be in lowercase, alphanumeric, kebab-case format. and follow DNS subdomain names","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[828,845,851],"startLine":54,"startColumn":9,"endLine":54,"endColumn":26}},{"message":"Invalid name for PipelineRun '-run-$(uid)'. Names should be in lowercase, alphanumeric, kebab-case format. and follow DNS subdomain names","rule":"no-invalid-name","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1356,1367,1377],"startLine":81,"startColumn":15,"endLine":81,"endColumn":26}},{"message":"TriggerTemplate 'no-invalid-name' references pipeline 'no-invalid-name', but the referenced pipeline cannot be found.","rule":"no-missing-resource","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1249,1264,1265],"startLine":77,"startColumn":17,"endLine":77,"endColumn":32}},{"message":"TriggerTemplate 'no-invalid-name' references pipeline 'no-invalid-name', but the referenced pipeline cannot be found.","rule":"no-missing-resource","level":"error","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1426,1441,1442],"startLine":84,"startColumn":17,"endLine":84,"endColumn":32}},{"message":"Task 'no-invalid-name' defines parameter 'foo', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1544,1559,1559],"startLine":92,"startColumn":7,"endLine":93,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'foo-', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1565,1581,1581],"startLine":93,"startColumn":7,"endLine":94,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'foo_', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1587,1603,1603],"startLine":94,"startColumn":7,"endLine":95,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'FOO_BAR', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1609,1628,1628],"startLine":95,"startColumn":7,"endLine":96,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'FooBar', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1634,1652,1652],"startLine":96,"startColumn":7,"endLine":97,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'fooBar', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1658,1676,1676],"startLine":97,"startColumn":7,"endLine":98,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'foo-bar', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1682,1701,1701],"startLine":98,"startColumn":7,"endLine":99,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'foo-1-bar', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1707,1728,1728],"startLine":99,"startColumn":7,"endLine":100,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'foo_1-bar', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1734,1755,1755],"startLine":100,"startColumn":7,"endLine":101,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter '1-foo', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1761,1782,1782],"startLine":101,"startColumn":7,"endLine":102,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter '-foo', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1788,1808,1808],"startLine":102,"startColumn":7,"endLine":103,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter '_foo', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1814,1830,1830],"startLine":103,"startColumn":7,"endLine":104,"endColumn":1}},{"message":"Task 'no-invalid-name' defines parameter 'foo$bar', but it's not used anywhere in the spec","rule":"no-unused-param","level":"warning","path":"./regression-tests/general/no-invalid-name.yaml","loc":{"range":[1836,1859,1859],"startLine":104,"startColumn":7,"endLine":105,"endColumn":1}}] \ No newline at end of file diff --git a/src/config.ts b/src/config.ts index cbd9b52..c2d9bdf 100644 --- a/src/config.ts +++ b/src/config.ts @@ -16,6 +16,7 @@ export class Config { private _refresh_cache: boolean; private _format: string; private _quiet: boolean; + private _custom: { [name: string]: string } | undefined; public constructor(cliConfig: any) { let user_tektonlintrc = path.resolve(cliConfig['config']); @@ -46,11 +47,16 @@ export class Config { this._format = cliConfig['format']; this._quiet = cliConfig['quiet']; + this._custom = this._rulesConfig['custom']; if (!fs.existsSync(this._cache_dir)) { fs.mkdirSync(this._cache_dir, { recursive: true }); } } + public get customRulesConfig() { + return this._custom; + } + public get rulesConfig() { return this._rulesConfig; } diff --git a/src/interfaces/common.ts b/src/interfaces/common.ts index ed92910..3b95809 100644 --- a/src/interfaces/common.ts +++ b/src/interfaces/common.ts @@ -62,6 +62,9 @@ export interface RulesConfig { [rule: string]: 'off' | 'warning' | 'error'; }; 'external-tasks': ExternalResource[]; + custom?: { + [name: string]: string; + }; } export type RuleReportFn = (message: string, node, prop?) => void; diff --git a/src/rule-loader.ts b/src/rule-loader.ts index 8e75d1e..add736c 100644 --- a/src/rule-loader.ts +++ b/src/rule-loader.ts @@ -1,4 +1,6 @@ -const rules = { +import { RulesConfig } from './interfaces/common.js'; + +const defaultRules = { 'no-resourceversion': (await import('./rules/no-resourceversion.js')).default, // no-duplicate-env @@ -63,4 +65,26 @@ const rules = { 'no-missing-hashbang': (await import('./rules/no-missing-hashbang.js')).default, }; -export default rules; +export class RuleLoader { + public static async getRules(config: RulesConfig) { + // load the custom rules + const custom = config.custom; + if (custom) { + for await (const [name, ruleFile] of Object.entries(custom)) { + const pluginRules = (await import(ruleFile)).default; + + pluginRules.rules.forEach((rule) => { + const fqRuleName = `${name}#${rule.name}`; + defaultRules[fqRuleName] = rule.ruleFn; + + // if not set by the user opt for default report level + if (!config.rules[fqRuleName]) { + config.rules[fqRuleName] = rule.reportDefault; + } + }); + } + } + + return defaultRules; + } +} diff --git a/src/rules.ts b/src/rules.ts index d6ad4a0..2b3f9fe 100644 --- a/src/rules.ts +++ b/src/rules.ts @@ -1,7 +1,7 @@ -import rules from './rule-loader.js'; +import { RuleLoader } from './rule-loader.js'; -import { logger } from './logger.js'; import { Tekton } from './interfaces/common.js'; +import { logger } from './logger.js'; const createReporter = (rule, config, reporter) => { const isError = config.rules[rule] && config.rules[rule] === 'error'; @@ -41,7 +41,9 @@ const parse = (docs): Tekton => { return tkn; }; -export function lint(docs, reporter, config) { +export async function lint(docs, reporter, config) { + const rules = await RuleLoader.getRules(config); + docs = docs.filter((doc) => doc && doc.metadata && doc.metadata.name); const tekton = parse(docs); diff --git a/src/runner.ts b/src/runner.ts index cfe6d55..6d308f0 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -40,7 +40,7 @@ export default async function runner(cfg: Config) { if (docs && docs.length > 0) { const reporter = new Reporter(docs); - return doLint( + return await doLint( docs.map((doc: any) => doc.content), reporter, cfg.rulesConfig,