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(core): template validation after synthesis #23951

Merged
merged 112 commits into from
Mar 27, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
112 commits
Select commit Hold shift + click to select a range
420efda
First draft
otaviomacedo Feb 1, 2023
d5e1d8b
Integrated with `App`
otaviomacedo Feb 3, 2023
c140db7
Checking whether checkov is installed
otaviomacedo Feb 7, 2023
aa421f4
Added kics validation
otaviomacedo Feb 7, 2023
0ef0c9f
Applying the cfn-guard validation
SankyRed Feb 16, 2023
69fe823
Moved validation to a separate phase
otaviomacedo Feb 17, 2023
aa0f21b
Added creation stack (class and id only)
otaviomacedo Feb 17, 2023
24bfd47
Removed unused interface
otaviomacedo Feb 20, 2023
d39ca9a
Taking a stab at the README
otaviomacedo Feb 20, 2023
46839e0
List of resources and constructs in ValidationViolationResourceAware …
otaviomacedo Feb 21, 2023
577b7bb
Fixed compilation error
otaviomacedo Feb 21, 2023
81c41c0
Added stack trace to the report
otaviomacedo Feb 21, 2023
f47bd62
Revert tree-metadata.ts to the original state
otaviomacedo Feb 21, 2023
c85d01c
creating an integration test package (private) and move the test plugins
corymhall Feb 21, 2023
9750076
some minor changes to validation
corymhall Feb 21, 2023
7b3205e
adding some initial unit tests
corymhall Feb 22, 2023
06ce483
adding some docs
corymhall Feb 22, 2023
07ed3c2
removing stdout option since it's not used
corymhall Feb 22, 2023
6963cd3
some work on allowing multiple plugins
corymhall Feb 22, 2023
c721590
Added more docs
otaviomacedo Feb 23, 2023
55ad8d9
New unit test
otaviomacedo Feb 23, 2023
506bac2
Removed logger
otaviomacedo Feb 23, 2023
dea5196
adding better trace info
corymhall Feb 23, 2023
66a23b1
making the validation report work per stack rather than per plugin
corymhall Feb 23, 2023
a1c79e6
Added more information to the README
otaviomacedo Feb 24, 2023
a370653
Fixed calls in the README
otaviomacedo Feb 24, 2023
1219c80
refactoring some things.
corymhall Feb 24, 2023
7764190
execute plugins at the stage level
corymhall Feb 24, 2023
858fbe9
fixing build and test
corymhall Feb 24, 2023
9072829
Comparing checksum after validation to prevent tampering
otaviomacedo Feb 27, 2023
098234e
Added note about modifying the cloud assembly
otaviomacedo Feb 27, 2023
bd64c77
removing parent from tree.json
corymhall Feb 27, 2023
b3b729d
adding validation plugins to analytics metadata
corymhall Feb 27, 2023
0791557
adding comments
corymhall Feb 27, 2023
eec9942
removing integ tests
corymhall Feb 28, 2023
5e0cc21
More unit tests
otaviomacedo Feb 28, 2023
a5be452
Hash and tree computation made lazy
otaviomacedo Feb 28, 2023
e10f76c
Refactoring: validation report logic split into a data structure and …
otaviomacedo Mar 3, 2023
b35a92e
Added test for multiple plugins
otaviomacedo Mar 3, 2023
893cdf5
Merge branch 'main' into otaviom/checkov-poc
otaviomacedo Mar 3, 2023
2abdbc9
Merge from main
otaviomacedo Mar 3, 2023
ebc0b48
Add an entry to the report when a plugin is not ready and keep going
otaviomacedo Mar 7, 2023
7573d93
Added `severity` to `ValidationViolation`
otaviomacedo Mar 7, 2023
bb4db9b
Remove spurious files
otaviomacedo Mar 7, 2023
dc8f59d
Removed spurious files
otaviomacedo Mar 7, 2023
ef820b5
Added doc to `validationPlugins` property
otaviomacedo Mar 7, 2023
c2b859d
User can choose report format
otaviomacedo Mar 7, 2023
659f7c3
ValidationContext -> IValidationContext
otaviomacedo Mar 7, 2023
df6c345
toJson -> formatJson; toString -> formatPrettyPrinted
otaviomacedo Mar 7, 2023
51b52e1
recommendation -> description
otaviomacedo Mar 7, 2023
bb6f6a1
resourceName -> resourceLogicalId
otaviomacedo Mar 7, 2023
e8cfc47
ValidationReport -> ValidationPluginReport
otaviomacedo Mar 7, 2023
1159fa4
Interfaces used only by the framework made private
otaviomacedo Mar 7, 2023
8311e7a
Merge branch 'main' into otaviom/checkov-poc
otaviomacedo Mar 7, 2023
4413d4f
Updated aws-cdk-lib dependencies
otaviomacedo Mar 7, 2023
cc3f8f0
Removed `isReady()` from `IValidationPlugin`
otaviomacedo Mar 7, 2023
1e9e47d
Merge branch 'main' into otaviom/checkov-poc
otaviomacedo Mar 7, 2023
cf34d40
Error message thrown by plugins added to the report
otaviomacedo Mar 8, 2023
dbdbea8
Merge branch 'main' into otaviom/checkov-poc
otaviomacedo Mar 8, 2023
7ac53dc
refactoring the construct trace logic into a separate class
corymhall Mar 8, 2023
db416ef
Updated yarn.lock
otaviomacedo Mar 8, 2023
34637cf
Merge branch 'main' into otaviom/checkov-poc
otaviomacedo Mar 8, 2023
f31d298
Removed pluginName from the fields that plugins need to return from v…
otaviomacedo Mar 8, 2023
60ae2a9
Updated aws-cdk-lib README
otaviomacedo Mar 8, 2023
d1bffce
Updated NOTICE file
otaviomacedo Mar 9, 2023
25cf6c2
Removed isReady mentions from the README and added a warning about th…
otaviomacedo Mar 9, 2023
120b393
Empty constructPath replaced with 'N/A' only in the pretty-printed fo…
otaviomacedo Mar 9, 2023
ee615d4
Updated README
otaviomacedo Mar 9, 2023
f192fde
fixing pretty print report
corymhall Mar 9, 2023
71944ab
get the template paths from the cloud assembly
corymhall Mar 10, 2023
973f000
fixing test
corymhall Mar 10, 2023
5fc3018
Controlling the format via the application context
otaviomacedo Mar 10, 2023
3b833e0
Added note about the feature being experimental
otaviomacedo Mar 10, 2023
5ced641
fixing trace
corymhall Mar 13, 2023
fa84b14
removing trailing space
corymhall Mar 13, 2023
ba5d710
Merge branch 'main' into otaviom/checkov-poc
otaviomacedo Mar 13, 2023
534d3fa
fixing trace test
corymhall Mar 14, 2023
bb8a089
fixing build
corymhall Mar 14, 2023
bd4e36a
Merge branch 'main' into otaviom/checkov-poc
corymhall Mar 15, 2023
7946cbb
adding ruleMetadata
corymhall Mar 15, 2023
3e1bcf1
adding rule metadata field
corymhall Mar 16, 2023
60da765
renaming everything to `PolicyValidation`
corymhall Mar 16, 2023
93d1255
Apply suggestions from code review
corymhall Mar 21, 2023
661b53a
addressing review comments
corymhall Mar 22, 2023
2e81c15
cleaning up logic
corymhall Mar 22, 2023
5568496
fixing yarn.lock
corymhall Mar 22, 2023
a7c0271
updates based on review
corymhall Mar 22, 2023
4e75df8
Clarified the use of context
otaviomacedo Mar 22, 2023
f2e0eba
adding tests for stages and fixing related issues
corymhall Mar 22, 2023
c7a1b34
update docstring
corymhall Mar 22, 2023
8ae9a0f
fixing tests for multiple stages
corymhall Mar 22, 2023
450b25c
more changes based on review
corymhall Mar 22, 2023
99aba79
refactoring trace cache methods
corymhall Mar 22, 2023
9b50d09
Updating report format
corymhall Mar 22, 2023
25e098a
fixing report
corymhall Mar 22, 2023
4fd03a1
renaming public api to beta1
corymhall Mar 22, 2023
8c021cc
Update packages/@aws-cdk/core/lib/private/synthesis.ts
otaviomacedo Mar 27, 2023
f5bf8fc
Mentioning the policy-validation-report.json file in the README
otaviomacedo Mar 27, 2023
6495177
Fixed test
otaviomacedo Mar 27, 2023
e5c4775
Added beta1 prefixes
otaviomacedo Mar 27, 2023
ccaf159
Added version field
otaviomacedo Mar 27, 2023
9e60ea0
Sync aws-cdk-lib readme with core readme
otaviomacedo Mar 27, 2023
b0dabdd
Merge branch 'main' into otaviom/checkov-poc
otaviomacedo Mar 27, 2023
cc4144c
Update packages/@aws-cdk/core/README.md
otaviomacedo Mar 27, 2023
85e2e0d
Update packages/@aws-cdk/core/README.md
otaviomacedo Mar 27, 2023
5c5a87c
Update packages/@aws-cdk/core/README.md
otaviomacedo Mar 27, 2023
dbe5cc7
Update packages/@aws-cdk/core/README.md
otaviomacedo Mar 27, 2023
3e73679
Apply suggestions from code review
otaviomacedo Mar 27, 2023
f15a688
fixing build
corymhall Mar 27, 2023
0a2e6b9
removing from feature flags
corymhall Mar 27, 2023
3ef2a76
fixing build
corymhall Mar 27, 2023
4746872
Merge branch 'main' into otaviom/checkov-poc
corymhall Mar 27, 2023
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
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@
"@aws-cdk/assertions-alpha/fs-extra/**",
"@aws-cdk/assertions/fs-extra",
"@aws-cdk/assertions/fs-extra/**",
"@aws-cdk/aws-iot-actions-alpha/case",
"@aws-cdk/aws-iot-actions-alpha/case/**",
"@aws-cdk/aws-codebuild/yaml",
"@aws-cdk/aws-codebuild/yaml/**",
"@aws-cdk/aws-codepipeline-actions/case",
Expand All @@ -98,6 +96,8 @@
"@aws-cdk/aws-eks/yaml/**",
"@aws-cdk/aws-events-targets/aws-sdk",
"@aws-cdk/aws-events-targets/aws-sdk/**",
"@aws-cdk/aws-iot-actions-alpha/case",
"@aws-cdk/aws-iot-actions-alpha/case/**",
"@aws-cdk/aws-iot-actions/case",
"@aws-cdk/aws-iot-actions/case/**",
"@aws-cdk/aws-s3-deployment/case",
Expand All @@ -110,12 +110,16 @@
"@aws-cdk/cloudformation-include/yaml/**",
"@aws-cdk/core/@balena/dockerignore",
"@aws-cdk/core/@balena/dockerignore/**",
"@aws-cdk/core/cross-spawn",
"@aws-cdk/core/cross-spawn/**",
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
"@aws-cdk/core/fs-extra",
"@aws-cdk/core/fs-extra/**",
"@aws-cdk/core/ignore",
"@aws-cdk/core/ignore/**",
"@aws-cdk/core/minimatch",
"@aws-cdk/core/minimatch/**",
"@aws-cdk/core/table",
"@aws-cdk/core/table/**",
"@aws-cdk/cx-api/semver",
"@aws-cdk/cx-api/semver/**",
"@aws-cdk/pipelines/aws-sdk",
Expand Down
16 changes: 16 additions & 0 deletions packages/@aws-cdk/core/lib/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { PRIVATE_CONTEXT_DEFAULT_STACK_SYNTHESIZER } from './private/private-con
import { addCustomSynthesis, ICustomSynthesis } from './private/synthesis';
import { IReusableStackSynthesizer } from './stack-synthesizers';
import { Stage } from './stage';
import { IValidationPlugin } from './validation/validation';

const APP_SYMBOL = Symbol.for('@aws-cdk/core.App');

Expand Down Expand Up @@ -118,6 +119,12 @@ export interface AppProps {
* @default - A `DefaultStackSynthesizer` with default settings
*/
readonly defaultStackSynthesizer?: IReusableStackSynthesizer;

/**
* TODO
* @default - TODO
*/
readonly validationPlugins?: IValidationPlugin[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super happy with using the word "validation" throughout, also because if conflates with CDK validations, and also because I think "policy" is more widely accepted as the correct term in this space.

How about:

Suggested change
readonly validationPlugins?: IValidationPlugin[];
readonly policyEnforcement?: IPolicyEnforcementPlugin[];

It reads as "you can enforce a policy using a set of policy enforcement plugins", which sounds right to me.

Then it can also be used as:

PolicyEnforcement.of(app).addPlugin(...)

I wish I could change that in cdk8s as well...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about PolicyValidation? I really want to avoid using the term enforcement anywhere because I don't want anyone to think that this feature is actually enforcing their policies.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also suggest PolicyCompliance - for your discretion :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iliapolo I renamed everything to PolicyValidation

}

/**
Expand Down Expand Up @@ -152,6 +159,11 @@ export class App extends Stage {
*/
public readonly _treeMetadata: boolean;

/**
* TODO docs
*/
public readonly validationPlugins: IValidationPlugin[] = [];

/**
* Initializes a CDK application.
* @param props initialization properties
Expand Down Expand Up @@ -186,6 +198,10 @@ export class App extends Stage {
process.once('beforeExit', () => this.synth());
}

if (props.validationPlugins) {
this.validationPlugins = props.validationPlugins;
}

this._treeMetadata = props.treeMetadata ?? true;
}

Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/core/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ export * from './cloudformation.generated';
export * from './feature-flags';
export * from './permissions-boundary';

export * from './validation';

// WARNING: Should not be exported, but currently is because of a bug. See the
// class description for more information.
export * from './private/intrinsic';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import * as cxapi from '@aws-cdk/cx-api';
import { DockerImageAssetLocation, DockerImageAssetSource, FileAssetLocation, FileAssetSource } from '../assets';
import { Stack } from '../stack';
import { Token } from '../token';
import { assertBound, StringSpecializer } from './_shared';
import { AssetManifestBuilder } from './asset-manifest-builder';
import { StackSynthesizer } from './stack-synthesizer';
import { ISynthesisSession, IReusableStackSynthesizer, IBoundStackSynthesizer } from './types';
import { App } from '../app';
import { DockerImageAssetLocation, DockerImageAssetSource, FileAssetLocation, FileAssetSource } from '../assets';
import { Stack } from '../stack';
import { Token } from '../token';

export const BOOTSTRAP_QUALIFIER_CONTEXT = '@aws-cdk/core:bootstrapQualifier';

Expand Down Expand Up @@ -425,6 +426,21 @@ export class DefaultStackSynthesizer extends StackSynthesizer implements IReusab
}

const templateAssetSource = this.synthesizeTemplate(session, this.lookupRoleArn);

const app = App.of(this.boundStack);
corymhall marked this conversation as resolved.
Show resolved Hide resolved
if (App.isApp(app)) {
// TODO - We probably need to find a better place to add the validationPlugins
for (const plugin of app.validationPlugins) {
plugin.validate(session, templateAssetSource).then(report => {
// eslint-disable-next-line no-console
console.log(report.toString());
}).finally(() => {
// eslint-disable-next-line no-console
console.log('Validation complete');
});
}
}

const templateAsset = this.addFileAsset(templateAssetSource);

const assetManifestId = this.assetManifest.emitManifest(this.boundStack, session, {
Expand Down
88 changes: 88 additions & 0 deletions packages/@aws-cdk/core/lib/validation/checkov.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import * as fs from 'fs';
import * as path from 'path';
import { sync } from 'cross-spawn';
import { IValidationPlugin, IValidation, ValidationContext, ValidationReport } from './validation';
import { FileAssetSource } from '../assets';
import { ISynthesisSession } from '../stack-synthesizers';

// NOTE: This class will eventually move out to a separate repository, but we're
// keeping it here for now to make it easier to iterate on.

// Design decisions:
// * We don't want to install checkov as a dependency of the CDK, so we'll just
// shell out to it.
// * Each entry in the checkov output is a separate violation.

/**
* TODO docs
*/
export class CheckovValiation implements IValidation {
/**
* TODO docs
*/
async validate(context: ValidationContext): Promise<void> {
if (!this.isCheckovInstalled()) {
throw new Error('Checkov is not installed. Install it by running "pip install checkov".');
}

await this.checkPolicies(context);
}

private isCheckovInstalled(): boolean {
corymhall marked this conversation as resolved.
Show resolved Hide resolved
const { status } = sync('checkov', ['--version'], {
encoding: 'utf-8',
stdio: 'pipe',
});

return status === 0;
}

private async checkPolicies(context: ValidationContext): Promise<void> {
const flags = [
'-f',
context.templatePath,
'-o',
'json',
];

const { status, output } = sync('checkov', flags, {
encoding: 'utf-8',
stdio: 'pipe',
});

const result = JSON.parse((output ?? []).join(''));

result.results.failed_checks.forEach((check: any) => {
context.report.addViolation({
fix: check.guideline,
recommendation: check.check_name,
ruleName: check.check_id,
violatingResource: {
resourceName: check.resource.split('.')[1],
templatePath: context.templatePath,
locations: check.check_result.evaluated_keys,
},
});
});

context.report.submit(status == 0 ? 'success' : 'failure');
}
}

/**
* TODO docs
*/
export class CheckovValidationPlugin implements IValidationPlugin {
private readonly validation = new CheckovValiation();

/**
* TODO docs
*/
async validate(session: ISynthesisSession, source: FileAssetSource): Promise<ValidationReport> {
const templateAbsolutePath = path.join(process.cwd(), session.outdir, source.fileName ?? '');
const template = JSON.parse(fs.readFileSync(templateAbsolutePath, { encoding: 'utf-8' }));
const validationContext = new ValidationContext('Checkov', template, templateAbsolutePath);
await this.validation.validate(validationContext);
return validationContext.report;
}
}
2 changes: 2 additions & 0 deletions packages/@aws-cdk/core/lib/validation/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './validation';
export * from './checkov';
102 changes: 102 additions & 0 deletions packages/@aws-cdk/core/lib/validation/kics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import * as fs from 'fs';
import * as path from 'path';
import { sync } from 'cross-spawn';
import { IValidation, IValidationPlugin, ValidationContext, ValidationReport } from './validation';
import { FileAssetSource } from '../assets';
import { ISynthesisSession } from '../stack-synthesizers';

// NOTE: This class will eventually move out to a separate repository, but we're
// keeping it here for now to make it easier to iterate on.

/**
* TODO docs
*/
export class KicsValidation implements IValidation {
/**
* TODO docs
*/
async validate(context: ValidationContext): Promise<void> {

await this.checkPolicies(context);
}

private async checkPolicies(context: ValidationContext): Promise<void> {
const [templateFolder, templateFileName] = this.splitPathAndFile(context.templatePath);

const flags = [
'run',
'-t',
`-v "${templateFolder}:/path"`,
'checkmarx/kics:latest',
'scan',
`-p /path/${templateFileName}`,
'-o "/path/"',
'--ci',
'--report-formats "json"',
];

// eslint-disable-next-line no-console
console.log(flags.join(' '));

// TODO This is not working yet. When I run the command directly in the
// terminal it works, but here I'm getting a docker error regarding the
// volume name.
const { status, output } = sync('docker', flags, {
encoding: 'utf-8',
stdio: 'pipe',
});

// eslint-disable-next-line no-console
console.log('status: ', status);

// eslint-disable-next-line no-console
console.log('stdout: ', output);

const foo = fs.readFileSync(path.join(templateFolder, 'results.json'), { encoding: 'utf-8' });

const results = JSON.parse(foo);

results.queries.forEach((query: any) => {
query.files.forEach((file: any) => {
context.report.addViolation({
fix: 'N/A',
recommendation: query.description,
ruleName: query.query_name,
violatingResource: {
resourceName: file.resource_name,
templatePath: file.file_name,
locations: [file.search_key],
},
});
});
});

context.report.submit(status == 0 ? 'success' : 'failure');
}

private splitPathAndFile(input: string): string[] {
const splitPath = input.split('/');
const fileName = splitPath.pop();
const filePath = splitPath.join('/');

return [filePath, fileName!];
}
}

/**
* TODO docs
*/
export class KicsValidationPlugin implements IValidationPlugin {
private readonly validation = new KicsValidation();

/**
* TODO docs
*/
async validate(session: ISynthesisSession, source: FileAssetSource): Promise<ValidationReport> {
const templateAbsolutePath = path.join(process.cwd(), session.outdir, source.fileName ?? '');
const template = JSON.parse(fs.readFileSync(templateAbsolutePath, { encoding: 'utf-8' }));
const validationContext = new ValidationContext('Kics', template, templateAbsolutePath);
await this.validation.validate(validationContext);
return validationContext.report;
}
}
Loading