Skip to content

Commit

Permalink
feat(core): add rule IDs to the analytics string (#25084)
Browse files Browse the repository at this point in the history
Adding rule IDs to the analytics string so we can track how much each individual rule is being used.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
otaviomacedo authored Apr 18, 2023
1 parent ac40aed commit 0c1e885
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 11 deletions.
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1397,6 +1397,10 @@ validate(context: ValidationContextBeta1): ValidationReportBeta1 {
}
```

In addition to the name, plugins may optionally report their version (`version`
property ) and a list of IDs of the rules they are going to evaluate (`ruleIds`
property).

Note that plugins are not allowed to modify anything in the cloud assembly. Any
attempt to do so will result in synthesis failure.

Expand Down
24 changes: 21 additions & 3 deletions packages/aws-cdk-lib/core/lib/private/runtime-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { IConstruct } from 'constructs';
import { App } from '../app';
import { Stack } from '../stack';
import { Stage } from '../stage';
import { IPolicyValidationPluginBeta1 } from '../validation';

const ALLOWED_FQN_PREFIXES = [
// SCOPES
Expand Down Expand Up @@ -55,9 +56,7 @@ function addValidationPluginInfo(stack: Stack, allConstructInfos: ConstructInfo[
allConstructInfos.push(...stage.policyValidationBeta1.map(
plugin => {
return {
// the fqn can be in the format of `package.module.construct`
// those get pulled out into separate fields
fqn: `policyValidation.${plugin.name}`,
fqn: pluginFqn(plugin),
version: plugin.version ?? '0.0.0',
};
},
Expand All @@ -67,6 +66,25 @@ function addValidationPluginInfo(stack: Stack, allConstructInfos: ConstructInfo[
} while (!done && stage);
}

/**
* Returns the fully-qualified name for a validation plugin, in the form:
*
* policyValidation.<plugin-name>[.<rule-ids>]
*
* where <rule-ids> is a pipe-separated list of rule IDs.
*/
function pluginFqn(plugin: IPolicyValidationPluginBeta1): string {
let components = [
'policyValidation',
plugin.name,
plugin.ruleIds?.join('|'),
];

return components
.filter(x => x != null)
.join('.');
}

/**
* For a given stack, walks the tree and finds the runtime info for all constructs within the tree.
* Returns the unique list of construct info present in the stack,
Expand Down
8 changes: 8 additions & 0 deletions packages/aws-cdk-lib/core/lib/validation/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ export interface IPolicyValidationPluginBeta1 {
*/
readonly version?: string;

/**
* The list of rule IDs that the plugin will evaluate. Used for analytics
* purposes.
*
* @default - No rule is reported
*/
readonly ruleIds?: string[];

/**
* The method that will be called by the CDK framework to perform
* validations. This is where the plugin will evaluate the CloudFormation
Expand Down
37 changes: 36 additions & 1 deletion packages/aws-cdk-lib/core/test/runtime-info.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from 'fs';
import * as path from 'path';
import { Construct } from 'constructs';
import { App, NestedStack, Stack, Stage } from '../lib';
import { App, NestedStack, Stack, Stage, IPolicyValidationPluginBeta1, PolicyViolationBeta1, PolicyValidationPluginReportBeta1, IPolicyValidationContextBeta1 } from '../lib';
import { constructInfoFromConstruct, constructInfoFromStack } from '../lib/private/runtime-info';

const JSII_RUNTIME_SYMBOL = Symbol.for('jsii.rtti');
Expand Down Expand Up @@ -157,6 +157,24 @@ describeTscSafe('constructInfoForStack', () => {
expect(fqns).not.toContain('@aws-cdk/test.TestNestedStackInsideStack');
expect(fqns).not.toContain('@aws-cdk/test.TestStageInsideStack');
});

test('return info from validator plugins', () => {
const validatedApp = new App({
policyValidationBeta1: [new FakePlugin('fake', [], '1.0.0', ['RULE_1', 'RULE_2'])],
});
const validatedStack = new Stack(validatedApp, 'ValidatedStack');
const constructInfos = constructInfoFromStack(validatedStack);

expect(constructInfos.map(info => info.fqn)).toContain('policyValidation.fake.RULE_1|RULE_2');
});

test('does not return info from validator plugins when no plugin is registered', () => {
const constructInfos = constructInfoFromStack(stack);

expect(constructInfos.map(info => info.fqn)).not.toEqual(expect.arrayContaining([
expect.stringMatching(/^policyValidation\./),
]));
});
});

class TestConstruct extends Construct {
Expand Down Expand Up @@ -191,3 +209,20 @@ function findParentPkgJson(dir: string, depth: number = 1, limit: number = 5): {

throw new Error(`No \`package.json\` file found within ${depth} parent directories`);
}

class FakePlugin implements IPolicyValidationPluginBeta1 {
constructor(
public readonly name: string,
private readonly violations: PolicyViolationBeta1[],
public readonly version?: string,
public readonly ruleIds?: string []) {
}

validate(_context: IPolicyValidationContextBeta1): PolicyValidationPluginReportBeta1 {
return {
success: this.violations.length === 0,
violations: this.violations,
pluginVersion: this.version,
};
}
}
15 changes: 8 additions & 7 deletions packages/aws-cdk-lib/core/test/validation/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ Policy Validation Report Summary
resourceLogicalId: 'DefaultResource',
templatePath: '/path/to/Stage1stack1DDED8B6C.template.json',
}],
}], '1.2.3'),
}]),
],
});
const stage1 = new core.Stage(app, 'Stage1', {
Expand All @@ -183,7 +183,7 @@ Policy Validation Report Summary
resourceLogicalId: 'DefaultResource',
templatePath: '/path/to/Stage1stack1DDED8B6C.template.json',
}],
}]),
}], '1.2.3'),
],
});
const stage2 = new core.Stage(app, 'Stage2', {
Expand Down Expand Up @@ -263,6 +263,7 @@ Policy Validation Report Summary
],
resourceLogicalId: 'DefaultResource',
description: 'do something',
version: '1.2.3',
},
{
pluginName: 'test-plugin4',
Expand Down Expand Up @@ -695,20 +696,18 @@ Policy Validation Report Summary
});

class FakePlugin implements core.IPolicyValidationPluginBeta1 {
private _version?: string;

constructor(
public readonly name: string,
private readonly violations: PolicyViolationBeta1[],
readonly version?: string) {
this._version = version;
public readonly version?: string,
public readonly ruleIds?: string []) {
}

validate(_context: core.IPolicyValidationContextBeta1): PolicyValidationPluginReportBeta1 {
return {
success: this.violations.length === 0,
violations: this.violations,
pluginVersion: this._version,
pluginVersion: this.version,
};
}
}
Expand Down Expand Up @@ -744,6 +743,7 @@ interface ValidationReportData {
description?: string;
resourceLogicalId: string;
severity?: string;
version?: string;
ruleMetadata?: { [key: string]: string };
}

Expand All @@ -770,6 +770,7 @@ const validationReport = (data: ValidationReportData[]) => {
expect.stringMatching(new RegExp('Validation Report')),
expect.stringMatching(new RegExp('-----------------')),
expect.stringMatching(new RegExp(`Plugin: ${d.pluginName}`)),
expect.stringMatching(new RegExp(`Version: ${d.version ?? 'N/A'}`)),
expect.stringMatching(new RegExp(`Status: ${d.status}`)),
expect.stringMatching(new RegExp('\(Violations\)')),
title,
Expand Down

0 comments on commit 0c1e885

Please sign in to comment.