Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: custom rules #106

Merged
merged 1 commit into from
Mar 8, 2024
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
5 changes: 4 additions & 1 deletion .tektonlintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

46 changes: 46 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

<!-- /TOC -->
Expand Down Expand Up @@ -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?

Expand Down
47 changes: 47 additions & 0 deletions custom_rules/README.md
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions custom_rules/my_rules.js
Original file line number Diff line number Diff line change
@@ -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
}
]
}
12 changes: 12 additions & 0 deletions custom_rules/package.json
Original file line number Diff line number Diff line change
@@ -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"
}
7 changes: 7 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions regression-tests/general/no-invalid-name.yaml.expect.json
Original file line number Diff line number Diff line change
@@ -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}}]
6 changes: 6 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 3 additions & 0 deletions src/interfaces/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 26 additions & 2 deletions src/rule-loader.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
}
}
8 changes: 5 additions & 3 deletions src/rules.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading