From b3dc864f0db0d7d7808c079841776439f4763201 Mon Sep 17 00:00:00 2001 From: ssube Date: Fri, 1 Nov 2019 22:02:25 -0500 Subject: [PATCH] feat: split rule and helpers, test rule --- config/rollup.js | 1 + examples/kubernetes-resources-some.yml | 3 + src/index.ts | 19 ++- src/rule.ts | 218 ------------------------- src/rule/SchemaRule.ts | 120 ++++++++++++++ src/rule/index.ts | 105 ++++++++++++ test/{ => rule}/TestRule.ts | 21 +-- test/rule/TestSchemaRule.ts | 98 +++++++++++ 8 files changed, 348 insertions(+), 237 deletions(-) delete mode 100644 src/rule.ts create mode 100644 src/rule/SchemaRule.ts create mode 100644 src/rule/index.ts rename test/{ => rule}/TestRule.ts (91%) create mode 100644 test/rule/TestSchemaRule.ts diff --git a/config/rollup.js b/config/rollup.js index b126a8b..1b88075 100644 --- a/config/rollup.js +++ b/config/rollup.js @@ -85,6 +85,7 @@ const bundle = { 'BaseError', 'ConsoleLogger', 'logWithLevel', + 'NullLogger', ], 'node_modules/js-yaml/index.js': [ 'DEFAULT_SAFE_SCHEMA', diff --git a/examples/kubernetes-resources-some.yml b/examples/kubernetes-resources-some.yml index cf6e7ee..26a15b1 100644 --- a/examples/kubernetes-resources-some.yml +++ b/examples/kubernetes-resources-some.yml @@ -31,13 +31,16 @@ spec: resources: limits: cpu: 200m + # missing memory requests: cpu: 100m + # same rule --- metadata: name: example + # missing labels spec: template: spec: diff --git a/src/index.ts b/src/index.ts index d82fcf8..44ae30b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,7 +3,8 @@ import { createLogger } from 'bunyan'; import { loadConfig } from './config'; import { CONFIG_ARGS_NAME, CONFIG_ARGS_PATH, parseArgs } from './config/args'; import { YamlParser } from './parser/YamlParser'; -import { loadRules, resolveRules, visitRules } from './rule'; +import { loadRules, resolveRules } from './rule'; +import { visitRules } from './rule/SchemaRule'; import { loadSource, writeSource } from './source'; import { VERSION_INFO } from './version'; import { VisitorContext } from './visitor/VisitorContext'; @@ -59,19 +60,19 @@ export async function main(argv: Array): Promise { await visitRules(ctx, activeRules, data); } - if (ctx.errors.length > 0) { - logger.error({ count: ctx.errors.length, errors: ctx.errors }, 'some rules failed'); - if (args.count) { - return Math.min(ctx.errors.length, STATUS_MAX); - } else { - return STATUS_ERROR; - } - } else { + if (ctx.errors.length === 0) { logger.info('all rules passed'); const output = parser.dump(...docs); await writeSource(args.dest, output); return STATUS_SUCCESS; } + + logger.error({ count: ctx.errors.length, errors: ctx.errors }, 'some rules failed'); + if (args.count) { + return Math.min(ctx.errors.length, STATUS_MAX); + } + + return STATUS_ERROR; } main(process.argv).then((status) => process.exit(status)).catch((err) => { diff --git a/src/rule.ts b/src/rule.ts deleted file mode 100644 index 9a32787..0000000 --- a/src/rule.ts +++ /dev/null @@ -1,218 +0,0 @@ -import { ValidateFunction } from 'ajv'; -import { applyDiff, diff } from 'deep-diff'; -import { JSONPath } from 'jsonpath-plus'; -import { cloneDeep, defaultTo, Dictionary, intersection, isNil } from 'lodash'; -import { LogLevel } from 'noicejs'; - -import { YamlParser } from './parser/YamlParser'; -import { readFileSync } from './source'; -import { ensureArray, hasItems } from './utils'; -import { friendlyError } from './utils/ajv'; -import { Visitor } from './visitor'; -import { VisitorContext } from './visitor/VisitorContext'; -import { VisitorError } from './visitor/VisitorError'; -import { VisitorResult } from './visitor/VisitorResult'; - -/* tslint:disable:no-any */ - -export interface RuleData { - // metadata - desc: string; - level: LogLevel; - name: string; - tags: Array; - // data - check: any; - filter?: any; - select: string; -} - -export interface RuleSelector { - excludeLevel: Array; - excludeName: Array; - excludeTag: Array; - includeLevel: Array; - includeName: Array; - includeTag: Array; -} - -export interface RuleSource { - definitions?: Dictionary; - name: string; - rules: Array; -} - -export interface RuleResult extends VisitorResult { - rule: Rule; -} - -export class Rule implements RuleData, Visitor { - public readonly check: ValidateFunction; - public readonly desc: string; - public readonly filter?: ValidateFunction; - public readonly level: LogLevel; - public readonly name: string; - public readonly select: string; - public readonly tags: Array; - - constructor(data: RuleData) { - this.desc = data.desc; - this.level = data.level; - this.name = data.name; - this.select = defaultTo(data.select, '$'); - this.tags = Array.from(data.tags); - - // copy schema objects - this.check = cloneDeep(data.check); - if (!isNil(data.filter)) { - this.filter = cloneDeep(data.filter); - } - } - - public async pick(ctx: VisitorContext, root: any): Promise> { - const scopes = JSONPath({ - json: root, - path: this.select, - }); - - if (isNil(scopes) || scopes.length === 0) { - ctx.logger.debug('no data selected'); - return []; - } - - return scopes; - } - - public async visit(ctx: VisitorContext, node: any): Promise { - ctx.logger.debug({ item: node, rule: this }, 'visiting node'); - - const check = ctx.compile(this.check); - const filter = this.compileFilter(ctx); - const errors: Array = []; - const result: RuleResult = { - changes: [], - errors, - rule: this, - }; - - if (filter(node)) { - ctx.logger.debug({ item: node }, 'checking item'); - if (!check(node) && !isNil(check.errors) && check.errors.length > 0) { - ctx.error(...Array.from(check.errors).map(friendlyError)); - } - } else { - ctx.logger.debug({ errors: filter.errors, item: node }, 'skipping item'); - } - - return result; - } - - protected compileFilter(ctx: VisitorContext): ValidateFunction { - if (isNil(this.filter)) { - return () => true; - } else { - return ctx.compile(this.filter); - } - } -} - -export function makeSelector(options: Partial) { - return { - excludeLevel: ensureArray(options.excludeLevel), - excludeName: ensureArray(options.excludeName), - excludeTag: ensureArray(options.excludeTag), - includeLevel: ensureArray(options.includeLevel), - includeName: ensureArray(options.includeName), - includeTag: ensureArray(options.includeTag), - }; -} - -export async function loadRules(paths: Array, ctx: VisitorContext): Promise> { - const parser = new YamlParser(); - const rules = []; - - for (const path of paths) { - const contents = await readFileSync(path, { - encoding: 'utf-8', - }); - - const docs = parser.parse(contents) as Array; - - for (const data of docs) { - if (!isNil(data.definitions)) { - ctx.addSchema(data.name, data.definitions); - } - - rules.push(...data.rules.map((it: RuleData) => new Rule(it))); - } - } - - return rules; -} - -export async function resolveRules(rules: Array, selector: RuleSelector): Promise> { - const activeRules = new Set(); - - for (const r of rules) { - if (selector.excludeLevel.includes(r.level)) { - continue; - } - - if (selector.excludeName.includes(r.name)) { - continue; - } - - const excludedTags = intersection(selector.excludeTag, r.tags); - if (excludedTags.length > 0) { - continue; - } - - if (selector.includeLevel.includes(r.level)) { - activeRules.add(r); - } - - if (selector.includeName.includes(r.name)) { - activeRules.add(r); - } - - const includedTags = intersection(selector.includeTag, r.tags); - if (includedTags.length > 0) { - activeRules.add(r); - } - } - - return Array.from(activeRules); -} - -export async function visitRules(ctx: VisitorContext, rules: Array, data: any): Promise { - for (const rule of rules) { - const items = await rule.pick(ctx, data); - for (const item of items) { - const itemCopy = cloneDeep(item); - const itemResult = await rule.visit(ctx, itemCopy); - - if (itemResult.errors.length > 0) { - ctx.logger.warn({ count: itemResult.errors.length, rule }, 'rule failed'); - ctx.mergeResult(itemResult); - continue; - } - - const itemDiff = diff(item, itemCopy); - if (hasItems(itemDiff)) { - ctx.logger.info({ - diff: itemDiff, - item, - rule: rule.name, - }, 'rule passed with modifications'); - - if (ctx.innerOptions.mutate) { - applyDiff(item, itemCopy); - } - } else { - ctx.logger.info({ rule: rule.name }, 'rule passed'); - } - } - } - - return ctx; -} diff --git a/src/rule/SchemaRule.ts b/src/rule/SchemaRule.ts new file mode 100644 index 0000000..b41bed2 --- /dev/null +++ b/src/rule/SchemaRule.ts @@ -0,0 +1,120 @@ +import { ValidateFunction } from 'ajv'; +import { applyDiff, diff } from 'deep-diff'; +import { JSONPath } from 'jsonpath-plus'; +import { cloneDeep, defaultTo, isNil } from 'lodash'; +import { LogLevel } from 'noicejs'; + +import { RuleData } from '.'; +import { hasItems } from '../utils'; +import { friendlyError } from '../utils/ajv'; +import { Visitor } from '../visitor'; +import { VisitorContext } from '../visitor/VisitorContext'; +import { VisitorError } from '../visitor/VisitorError'; +import { VisitorResult } from '../visitor/VisitorResult'; + +export interface RuleResult extends VisitorResult { + rule: SchemaRule; +} + +export class SchemaRule implements RuleData, Visitor { + public readonly check: ValidateFunction; + public readonly desc: string; + public readonly filter?: ValidateFunction; + public readonly level: LogLevel; + public readonly name: string; + public readonly select: string; + public readonly tags: Array; + + constructor(data: RuleData) { + this.desc = data.desc; + this.level = data.level; + this.name = data.name; + this.select = defaultTo(data.select, '$'); + this.tags = Array.from(data.tags); + + // copy schema objects + this.check = cloneDeep(data.check); + if (!isNil(data.filter)) { + this.filter = cloneDeep(data.filter); + } + } + + public async pick(ctx: VisitorContext, root: any): Promise> { + const scopes = JSONPath({ + json: root, + path: this.select, + }); + + if (hasItems(scopes)) { + return scopes; + } + + ctx.logger.debug('no data selected'); + return []; + } + + public async visit(ctx: VisitorContext, node: any): Promise { + ctx.logger.debug({ item: node, rule: this }, 'visiting node'); + + const check = ctx.compile(this.check); + const filter = this.compileFilter(ctx); + const errors: Array = []; + const result: RuleResult = { + changes: [], + errors, + rule: this, + }; + + if (filter(node)) { + ctx.logger.debug({ item: node }, 'checking item'); + if (!check(node) && !isNil(check.errors) && check.errors.length > 0) { + ctx.error(...Array.from(check.errors).map(friendlyError)); + } + } else { + ctx.logger.debug({ errors: filter.errors, item: node }, 'skipping item'); + } + + return result; + } + + protected compileFilter(ctx: VisitorContext): ValidateFunction { + if (isNil(this.filter)) { + return () => true; + } else { + return ctx.compile(this.filter); + } + } +} + +export async function visitRules(ctx: VisitorContext, rules: Array, data: any): Promise { + for (const rule of rules) { + const items = await rule.pick(ctx, data); + for (const item of items) { + const itemResult = cloneDeep(item); + const ruleResult = await rule.visit(ctx, itemResult); + + if (ruleResult.errors.length > 0) { + ctx.logger.warn({ count: ruleResult.errors.length, rule }, 'rule failed'); + ctx.mergeResult(ruleResult); + continue; + } + + const itemDiff = diff(item, itemResult); + if (hasItems(itemDiff)) { + ctx.logger.info({ + diff: itemDiff, + item, + rule: rule.name, + }, 'rule passed with modifications'); + + if (ctx.innerOptions.mutate) { + applyDiff(item, itemResult); + } + } else { + ctx.logger.info({ rule: rule.name }, 'rule passed'); + } + } + } + + return ctx; +} diff --git a/src/rule/index.ts b/src/rule/index.ts new file mode 100644 index 0000000..067d479 --- /dev/null +++ b/src/rule/index.ts @@ -0,0 +1,105 @@ +import { Dictionary, intersection, isNil } from 'lodash'; +import { LogLevel } from 'noicejs'; + +import { YamlParser } from '../parser/YamlParser'; +import { readFileSync } from '../source'; +import { ensureArray } from '../utils'; +import { VisitorContext } from '../visitor/VisitorContext'; +import { SchemaRule } from './SchemaRule'; + +/* tslint:disable:no-any */ + +export interface RuleData { + // metadata + desc: string; + level: LogLevel; + name: string; + tags: Array; + // data + check: any; + filter?: any; + select: string; +} + +export interface RuleSelector { + excludeLevel: Array; + excludeName: Array; + excludeTag: Array; + includeLevel: Array; + includeName: Array; + includeTag: Array; +} + +export interface RuleSource { + definitions?: Dictionary; + name: string; + rules: Array; +} + +export function makeSelector(options: Partial) { + return { + excludeLevel: ensureArray(options.excludeLevel), + excludeName: ensureArray(options.excludeName), + excludeTag: ensureArray(options.excludeTag), + includeLevel: ensureArray(options.includeLevel), + includeName: ensureArray(options.includeName), + includeTag: ensureArray(options.includeTag), + }; +} + +export async function loadRules(paths: Array, ctx: VisitorContext): Promise> { + const parser = new YamlParser(); + const rules = []; + + for (const path of paths) { + const contents = await readFileSync(path, { + encoding: 'utf-8', + }); + + const docs = parser.parse(contents) as Array; + + for (const data of docs) { + if (!isNil(data.definitions)) { + ctx.addSchema(data.name, data.definitions); + } + + rules.push(...data.rules.map((it: RuleData) => new SchemaRule(it))); + } + } + + return rules; +} + +export async function resolveRules(rules: Array, selector: RuleSelector): Promise> { + const activeRules = new Set(); + + for (const r of rules) { + if (selector.excludeLevel.includes(r.level)) { + continue; + } + + if (selector.excludeName.includes(r.name)) { + continue; + } + + const excludedTags = intersection(selector.excludeTag, r.tags); + if (excludedTags.length > 0) { + continue; + } + + if (selector.includeLevel.includes(r.level)) { + activeRules.add(r); + } + + if (selector.includeName.includes(r.name)) { + activeRules.add(r); + } + + const includedTags = intersection(selector.includeTag, r.tags); + if (includedTags.length > 0) { + activeRules.add(r); + } + } + + return Array.from(activeRules); +} diff --git a/test/TestRule.ts b/test/rule/TestRule.ts similarity index 91% rename from test/TestRule.ts rename to test/rule/TestRule.ts index 21b2b7d..5f9e797 100644 --- a/test/TestRule.ts +++ b/test/rule/TestRule.ts @@ -2,25 +2,26 @@ import { expect } from 'chai'; import { ConsoleLogger } from 'noicejs'; import { mock, spy } from 'sinon'; -import { makeSelector, resolveRules, Rule, visitRules } from '../src/rule'; -import { VisitorContext } from '../src/visitor/VisitorContext'; -import { describeLeaks, itLeaks } from './helpers/async'; +import { makeSelector, resolveRules } from '../../src/rule'; +import { SchemaRule, visitRules } from '../../src/rule/SchemaRule'; +import { VisitorContext } from '../../src/visitor/VisitorContext'; +import { describeLeaks, itLeaks } from '../helpers/async'; -const TEST_RULES = [new Rule({ +const TEST_RULES = [new SchemaRule({ check: {}, desc: '', level: 'info', name: 'foo', select: '$', tags: ['all', 'foo'], -}), new Rule({ +}), new SchemaRule({ check: {}, desc: '', level: 'warn', name: 'bar', select: '$', tags: ['all', 'test'], -}), new Rule({ +}), new SchemaRule({ check: {}, desc: '', level: 'warn', @@ -111,7 +112,7 @@ describeLeaks('rule visitor', async () => { logger: new ConsoleLogger(), }); const data = {}; - const rule = new Rule({ + const rule = new SchemaRule({ check: {}, desc: '', level: 'info', @@ -143,7 +144,7 @@ describeLeaks('rule visitor', async () => { logger: new ConsoleLogger(), }); const data = {}; - const rule = new Rule({ + const rule = new SchemaRule({ check: {}, desc: '', level: 'info', @@ -180,7 +181,7 @@ describeLeaks('rule visitor', async () => { const data = { foo: 3, }; - const rule = new Rule({ + const rule = new SchemaRule({ check: {}, desc: '', level: 'info', @@ -207,7 +208,7 @@ describeLeaks('rule visitor', async () => { const data = { foo: 3, }; - const rule = new Rule({ + const rule = new SchemaRule({ check: {}, desc: '', filter: { diff --git a/test/rule/TestSchemaRule.ts b/test/rule/TestSchemaRule.ts new file mode 100644 index 0000000..64cc192 --- /dev/null +++ b/test/rule/TestSchemaRule.ts @@ -0,0 +1,98 @@ +import { expect } from 'chai'; +import { NullLogger } from 'noicejs'; +import { stub } from 'sinon'; + +import { SchemaRule } from '../../src/rule/SchemaRule'; +import { VisitorContext } from '../../src/visitor/VisitorContext'; +import { describeLeaks, itLeaks } from '../helpers/async'; + +const TEST_NAME = 'test-rule'; + +describeLeaks('schema rule', async () => { + itLeaks('should pick items from the root', async () => { + const ctx = new VisitorContext({ + innerOptions: { + coerce: false, + defaults: false, + mutate: false, + }, + logger: NullLogger.global, + }); + const rule = new SchemaRule({ + check: undefined, + desc: TEST_NAME, + level: 'info', + name: TEST_NAME, + select: '$.foo', + tags: [], + }); + const results = await rule.pick(ctx, { + foo: [1, 2, 3], + }); + expect(Array.isArray(results)).to.equal(true); + }); + + itLeaks('should visit selected items', async () => { + const ctx = new VisitorContext({ + innerOptions: { + coerce: false, + defaults: false, + mutate: false, + }, + logger: NullLogger.global, + }); + + const check = {}; + const checkSpy = stub().returns(true); + const filter = {}; + const filterSpy = stub().returns(true); + ctx.compile = stub().onFirstCall().returns(checkSpy).onSecondCall().returns(filterSpy); + + const rule = new SchemaRule({ + check, + desc: TEST_NAME, + filter, + level: 'info', + name: TEST_NAME, + select: '$.foo', + tags: [], + }); + + const data = {}; + await rule.visit(ctx, data); + + expect(filterSpy, 'filter spy should have been called with data').to.have.callCount(1).and.been.calledWithExactly(data); + expect(checkSpy, 'check spy should have been called with data').to.have.callCount(1).and.been.calledWithExactly(data); + }); + + itLeaks('should skip filtered items', async () => { + const ctx = new VisitorContext({ + innerOptions: { + coerce: false, + defaults: false, + mutate: false, + }, + logger: NullLogger.global, + }); + + const checkSpy = stub().throws(new Error('check spy error')); + const filterSpy = stub().returns(false); + ctx.compile = stub().onFirstCall().returns(checkSpy).onSecondCall().returns(filterSpy); + + const rule = new SchemaRule({ + check: undefined, + desc: TEST_NAME, + filter: {}, + level: 'info', + name: TEST_NAME, + select: '$.foo', + tags: [], + }); + + const data = {}; + await rule.visit(ctx, data); + + expect(filterSpy, 'filter spy should have been called with data').to.have.callCount(1).and.been.calledWithExactly(data); + expect(checkSpy, 'check spy should not have been called').to.have.callCount(0); + }); +});