From eb1fdd3f304abd3f17608869aea7979c3de25a3f Mon Sep 17 00:00:00 2001 From: ssube Date: Sun, 23 Jun 2019 22:48:07 -0500 Subject: [PATCH] formalize visitor, results, etc --- config/rollup.js | 3 +- rules/kubernetes.yml | 4 +-- src/index.ts | 27 ++++++++++-------- src/rule.ts | 63 +++++++++++++++++++++++++----------------- src/visitor/context.ts | 12 ++++++-- src/visitor/index.ts | 15 ++++++++-- src/visitor/result.ts | 4 +++ 7 files changed, 84 insertions(+), 44 deletions(-) create mode 100644 src/visitor/result.ts diff --git a/config/rollup.js b/config/rollup.js index da6ed90..24a02b9 100644 --- a/config/rollup.js +++ b/config/rollup.js @@ -36,6 +36,7 @@ export default { commonjs({ namedExports: { 'node_modules/deep-diff/index.js': [ + 'applyDiff', 'diff', ], 'node_modules/lodash/lodash.js': [ @@ -63,4 +64,4 @@ export default { rollupCommonJSResolveHack: true, }), ], -}; \ No newline at end of file +}; diff --git a/rules/kubernetes.yml b/rules/kubernetes.yml index 6b90a59..a3b162b 100644 --- a/rules/kubernetes.yml +++ b/rules/kubernetes.yml @@ -56,7 +56,7 @@ rules: limits: type: object properties: - cpu: &resources-cpu + cpu: *resources-cpu # ensure the limits aren't *too* low check: @@ -99,4 +99,4 @@ rules: properties: replica: type: number - minimum: 1 \ No newline at end of file + minimum: 1 diff --git a/src/index.ts b/src/index.ts index 1013b4e..a29581b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,5 @@ import { createLogger } from 'bunyan'; -import { diff } from 'deep-diff'; +import { applyDiff, diff } from 'deep-diff'; import { cloneDeep } from 'lodash'; import { Options, usage } from 'yargs'; @@ -117,19 +117,24 @@ export async function main(argv: Array): Promise { const activeRules = await resolveRules(rules, args as any); for (const rule of activeRules) { - const workingCopy = cloneDeep(data); - const ruleErrors = await rule.visit(ctx, workingCopy); + const items = await rule.pick(ctx, data); + for (const item of items) { + const itemCopy = cloneDeep(item); + const itemResult = await rule.visit(ctx, itemCopy); - if (ruleErrors > 0) { - logger.warn({ rule }, 'rule failed'); - } else { - const ruleDiff = diff(data, workingCopy); - if (Array.isArray(ruleDiff) && ruleDiff.length > 0) { - logger.info({ diff: ruleDiff, rule }, 'rule passed with modifications'); + if (itemResult.errors.length > 0) { + logger.warn({ count: itemResult.errors.length, rule }, 'rule failed'); - data = workingCopy; + ctx.mergeResult(itemResult); } else { - logger.info({ rule }, 'rule passed'); + const itemDiff = diff(item, itemCopy); + if (Array.isArray(itemDiff) && itemDiff.length > 0) { + logger.info({ diff: itemDiff, item, rule }, 'rule passed with modifications'); + + applyDiff(item, itemDiff); + } else { + logger.info({ rule }, 'rule passed'); + } } } } diff --git a/src/rule.ts b/src/rule.ts index 670c7a1..37ff39b 100644 --- a/src/rule.ts +++ b/src/rule.ts @@ -6,6 +6,7 @@ import { YamlParser } from 'src/parser/YamlParser'; import { readFileSync } from 'src/source'; import { Visitor } from 'src/visitor'; import { VisitorContext } from 'src/visitor/context'; +import { VisitorResult } from 'src/visitor/result'; export interface RuleData { // metadata @@ -92,7 +93,11 @@ export async function resolveRules(rules: Array, selector: RuleSelector): return Array.from(activeRules); } -export class Rule implements RuleData, Visitor { +export interface RuleResult extends VisitorResult { + rule: Rule; +} + +export class Rule implements RuleData, Visitor { public readonly check: any; public readonly desc: string; public readonly filter?: any; @@ -115,40 +120,48 @@ export class Rule implements RuleData, Visitor { } } - public async visit(ctx: VisitorContext, node: any): Promise { - const check = ctx.ajv.compile(this.check); - const filter = this.compileFilter(ctx); + public async pick(ctx: VisitorContext, root: any): Promise> { const scopes = JSONPath({ - json: node, + json: root, path: this.select, }); if (isNil(scopes) || scopes.length === 0) { ctx.logger.debug('no data selected'); - return 0; + return []; } - for (const item of scopes) { - ctx.logger.debug({ item }, 'filtering item'); - if (filter(item)) { - ctx.logger.debug({ item }, 'checking item') - if (!check(item)) { - const errors = Array.from(check.errors); - ctx.logger.warn({ - errors, - name: this.name, - item, - rule: this, - }, 'rule failed on item'); - ctx.errors.push(...errors); - return errors.length; - } - } else { - ctx.logger.debug({ errors: filter.errors, item }, 'skipping item'); + return scopes; + } + + public async visit(ctx: VisitorContext, node: any): Promise { + ctx.logger.debug({ item: node, rule: this}, 'visiting node'); + + const check = ctx.ajv.compile(this.check); + const filter = this.compileFilter(ctx); + const result: RuleResult = { + changes: [], + errors: [], + rule: this, + }; + + if (filter(node)) { + ctx.logger.debug({ item: node }, 'checking item') + if (!check(node)) { + const errors = Array.from(check.errors); + ctx.logger.warn({ + errors, + name: this.name, + item: node, + rule: this, + }, 'rule failed on item'); + result.errors.push(...errors); } + } else { + ctx.logger.debug({ errors: filter.errors, item: node }, 'skipping item'); } - return 0; + return result; } protected compileFilter(ctx: VisitorContext): any { @@ -158,4 +171,4 @@ export class Rule implements RuleData, Visitor { return ctx.ajv.compile(this.filter); } } -} \ No newline at end of file +} diff --git a/src/visitor/context.ts b/src/visitor/context.ts index 875e515..2deee38 100644 --- a/src/visitor/context.ts +++ b/src/visitor/context.ts @@ -1,13 +1,15 @@ import * as Ajv from 'ajv'; import { Logger } from 'noicejs'; +import { VisitorResult } from 'src/visitor/result'; + export interface VisitorContextOptions { coerce: boolean; defaults: boolean; logger: Logger; } -export class VisitorContext { +export class VisitorContext implements VisitorContextOptions, VisitorResult { public readonly ajv: any; public readonly changes: Array; public readonly coerce: boolean; @@ -31,4 +33,10 @@ export class VisitorContext { this.logger.error(options, msg); this.errors.push(options || msg); } -} \ No newline at end of file + + public mergeResult(other: VisitorResult): this { + this.changes.push(...other.changes); + this.errors.push(...other.errors); + return this; + } +} diff --git a/src/visitor/index.ts b/src/visitor/index.ts index 27ca6f3..42dfe41 100644 --- a/src/visitor/index.ts +++ b/src/visitor/index.ts @@ -1,5 +1,14 @@ import { VisitorContext } from 'src/visitor/context'; +import { VisitorResult } from 'src/visitor/result'; -export interface Visitor { - visit(ctx: VisitorContext, node: any): Promise; -} \ No newline at end of file +export interface Visitor { + /** + * Select nodes eligible to be visited. + **/ + pick(ctx: VisitorContext, root: any): Promise>; + + /** + * Visit a node. + */ + visit(ctx: VisitorContext, node: any): Promise; +} diff --git a/src/visitor/result.ts b/src/visitor/result.ts new file mode 100644 index 0000000..7bb2dce --- /dev/null +++ b/src/visitor/result.ts @@ -0,0 +1,4 @@ +export interface VisitorResult { + changes: Array; + errors: Array; +}