diff --git a/README.md b/README.md index 30ce17098c..a14a55a491 100644 --- a/README.md +++ b/README.md @@ -291,6 +291,69 @@ You would see the following error on synth/deploy +## Suppressing Rule Validation Failures + +When a rule validation fails it is handled similarly to a rule violation, and can be suppressed in the same manner. The `ID` for a rule failure is `CdkNagValidationFailure`. + +If a rule is suppressed in a non-granular manner (i.e. `appliesTo` is not set, see example 1 above) then validation failures on that rule are also suppressed. + +Validation failure suppression respects any applied [Suppression Ignore Conditions](#conditionally-ignoring-suppressions) + +
+ Example 1) Suppress all Validation Failures on a Resource + +```typescript +import { SecurityGroup, Vpc, Peer, Port } from 'aws-cdk-lib/aws-ec2'; +import { Stack, StackProps } from 'aws-cdk-lib'; +import { Construct } from 'constructs'; +import { NagSuppressions } from 'cdk-nag'; + +export class CdkTestStack extends Stack { + constructor(scope: Construct, id: string, props?: StackProps) { + super(scope, id, props); + const test = new SecurityGroup(this, 'test', { + vpc: new Vpc(this, 'vpc'), + }); + test.addIngressRule(Peer.anyIpv4(), Port.allTraffic()); + NagSuppressions.addResourceSuppressions(test, [ + { id: 'CdkNagValidationFailure', reason: 'lorem ipsum' }, + ]); + } +} +``` + +
+ +
+ Example 2) Granular Suppression of Validation Failures +Validation failures can be suppressed for individual rules by using `appliesTo` to list the desired rules + +```typescript +import { SecurityGroup, Vpc, Peer, Port } from 'aws-cdk-lib/aws-ec2'; +import { Stack, StackProps } from 'aws-cdk-lib'; +import { Construct } from 'constructs'; +import { NagSuppressions } from 'cdk-nag'; + +export class CdkTestStack extends Stack { + constructor(scope: Construct, id: string, props?: StackProps) { + super(scope, id, props); + const test = new SecurityGroup(this, 'test', { + vpc: new Vpc(this, 'vpc'), + }); + test.addIngressRule(Peer.anyIpv4(), Port.allTraffic()); + NagSuppressions.addResourceSuppressions(test, [ + { + id: 'CdkNagValidationFailure', + reason: 'lorem ipsum', + appliesTo: ['AwsSolutions-L1'], + }, + ]); + } +} +``` + +
+ ## Suppressing `aws-cdk-lib/pipelines` Violations The [aws-cdk-lib/pipelines.CodePipeline](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.pipelines.CodePipeline.html) construct and its child constructs are not guaranteed to be "Visited" by `Aspects`, as they are not added during the "Construction" phase of the [cdk lifecycle](https://docs.aws.amazon.com/cdk/v2/guide/apps.html#lifecycle). Because of this behavior, you may experience problems such as rule violations not appearing or the inability to suppress violations on these constructs. diff --git a/src/nag-logger.ts b/src/nag-logger.ts index fc95d306d2..0c734725ef 100644 --- a/src/nag-logger.ts +++ b/src/nag-logger.ts @@ -160,7 +160,7 @@ export class AnnotationLogger implements INagLogger { const information = `'${data.ruleId}' threw an error during validation. This is generally caused by a parameter referencing an intrinsic function. You can suppress the "${VALIDATION_FAILURE_ID}" to get rid of this error. For more details enable verbose logging.'`; const message = this.createMessage( VALIDATION_FAILURE_ID, - '', + data.ruleId, information, data.errorMessage, this.verbose @@ -171,7 +171,7 @@ export class AnnotationLogger implements INagLogger { if (this.logIgnores === true) { const message = this.createMessage( this.suppressionId, - '', + data.ruleId, `${VALIDATION_FAILURE_ID} was triggered but suppressed.`, data.errorSuppressionReason, this.verbose diff --git a/src/nag-pack.ts b/src/nag-pack.ts index 8ac2af54f6..14b776a9a2 100644 --- a/src/nag-pack.ts +++ b/src/nag-pack.ts @@ -199,11 +199,12 @@ export abstract class NagPack implements IAspect { } catch (error) { const reason = this.ignoreRule( allSuppressions, - VALIDATION_FAILURE_ID, + ruleId, '', params.node, params.level, - params.ignoreSuppressionCondition + params.ignoreSuppressionCondition, + true ); if (reason) { this.loggers.forEach((t) => @@ -230,6 +231,7 @@ export abstract class NagPack implements IAspect { * @param ruleId The id of the rule to ignore. * @param resource The resource being evaluated. * @param findingId The id of the finding that is being checked. + * @param validationFailure Whether the rule is being checked due to a validation failure. * @returns The reason the rule was ignored, or an empty string. */ protected ignoreRule( @@ -238,10 +240,20 @@ export abstract class NagPack implements IAspect { findingId: string, resource: CfnResource, level: NagMessageLevel, - ignoreSuppressionCondition?: INagSuppressionIgnore + ignoreSuppressionCondition?: INagSuppressionIgnore, + validationFailure: boolean = false ): string { for (let suppression of suppressions) { - if (NagSuppressionHelper.doesApply(suppression, ruleId, findingId)) { + if ( + NagSuppressionHelper.doesApply(suppression, ruleId, findingId) || + // If this is marked as an exception, also check for a suppression on VALIDATION_FAILURE_ID + (validationFailure && + NagSuppressionHelper.doesApply( + suppression, + VALIDATION_FAILURE_ID, + ruleId + )) + ) { const ignoreMessage = new SuppressionIgnoreOr( this.userGlobalSuppressionIgnore ?? new SuppressionIgnoreNever(), this.packGlobalSuppressionIgnore ?? new SuppressionIgnoreNever(), diff --git a/test/Conditions.test.ts b/test/Conditions.test.ts index 4faab68e3c..d7d04f6687 100644 --- a/test/Conditions.test.ts +++ b/test/Conditions.test.ts @@ -15,6 +15,7 @@ import { SuppressionIgnoreOr, } from '../src'; import { TestPack } from './rules/utils'; +import { expectMessages } from './test-utils'; describe('Rule Suppression Condition Core Functionality', () => { const IGNORE = new SuppressionIgnoreAlways('IGNORED.'); @@ -37,22 +38,10 @@ describe('Rule Suppression Condition Core Functionality', () => { Aspects.of(stack).add(testPack); new CfnResource(stack, 'nice', { type: 'AWS::Infinidash::Meme' }); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'was ignored for the following reason(s)' - ), - }), - }) - ); + expectMessages(messages, { + containing: ['Test-Condition'], + notContaining: ['was ignored for the following reason(s)'], + }); }); test('Not ignored with suppression', () => { const testPack = new TestPack( @@ -74,22 +63,12 @@ describe('Rule Suppression Condition Core Functionality', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'was ignored for the following reason(s)' - ), - }), - }) - ); + expectMessages(messages, { + notContaining: [ + 'Test-Condition', + 'was ignored for the following reason(s)', + ], + }); }); test('Ignored no suppression', () => { const testPack = new TestPack( @@ -105,22 +84,10 @@ describe('Rule Suppression Condition Core Functionality', () => { new CfnResource(stack, 'nice', { type: 'AWS::Infinidash::Meme' }); Aspects.of(stack).add(testPack); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'was ignored for the following reason(s)' - ), - }), - }) - ); + expectMessages(messages, { + containing: ['Test-Condition'], + notContaining: ['was ignored for the following reason(s)'], + }); }); test('Ignored with suppression', () => { const testPack = new TestPack( @@ -142,22 +109,9 @@ describe('Rule Suppression Condition Core Functionality', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'was ignored for the following reason(s)' - ), - }), - }) - ); + expectMessages(messages, { + containing: ['Test-Condition', 'was ignored for the following reason(s)'], + }); }); }); @@ -189,20 +143,9 @@ describe('Prebuilt Rule Suppression Conditions', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('IGNORED.\n\tIGNORED.'), - }), - }) - ); + expectMessages(messages, { + containing: ['Test-Condition', 'IGNORED.\n\tIGNORED.'], + }); }); test('Should Not Ignore Suppression', () => { const testPack = new TestPack( @@ -224,20 +167,9 @@ describe('Prebuilt Rule Suppression Conditions', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('IGNORED.'), - }), - }) - ); + expectMessages(messages, { + notContaining: ['Test-Condition', 'IGNORED.'], + }); }); }); describe('IgnoreOr', () => { @@ -261,20 +193,9 @@ describe('Prebuilt Rule Suppression Conditions', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('IGNORED.'), - }), - }) - ); + expectMessages(messages, { + containing: ['Test-Condition', 'IGNORED.'], + }); }); test('Should Not Ignore Suppression', () => { const testPack = new TestPack( @@ -296,22 +217,12 @@ describe('Prebuilt Rule Suppression Conditions', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'was ignored for the following reason(s)' - ), - }), - }) - ); + expectMessages(messages, { + notContaining: [ + 'Test-Condition', + 'was ignored for the following reason(s)', + ], + }); }); }); describe('SuppressionIgnoreErrors', () => { @@ -336,20 +247,9 @@ describe('Prebuilt Rule Suppression Conditions', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('categorized as an ERROR'), - }), - }) - ); + expectMessages(messages, { + containing: ['Test-Condition', 'categorized as an ERROR'], + }); }); test('Should Not Ignore Suppression', () => { const testPack = new TestPack( @@ -372,20 +272,9 @@ describe('Prebuilt Rule Suppression Conditions', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Test-Condition'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('categorized as an ERROR'), - }), - }) - ); + expectMessages(messages, { + notContaining: ['Test-Condition', 'categorized as an ERROR'], + }); }); }); }); diff --git a/test/Engine.test.ts b/test/Engine.test.ts index acc9b5161f..bfde6101c8 100644 --- a/test/Engine.test.ts +++ b/test/Engine.test.ts @@ -30,7 +30,9 @@ import { NagRuleCompliance, NagRules, NagSuppressions, + SuppressionIgnoreAlways, } from '../src'; +import { expectMessages } from './test-utils'; describe('Rule suppression system', () => { test('Test single rule suppression', () => { @@ -45,13 +47,7 @@ describe('Rule suppression system', () => { rules_to_suppress: [{ id: 'AwsSolutions-EC23', reason: 'lorem ipsum' }], }); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-EC23:'), - }), - }) - ); + expectMessages(messages, { notContaining: ['AwsSolutions-EC23:'] }); }); test('Fine grained permission cannot be added via rule id [resource]', () => { const stack = new Stack(); @@ -108,20 +104,12 @@ describe('Rule suppression system', () => { true ); const { messages } = SynthUtils.synthesize(stack); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-IAM5[Action::s3:*]:'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-IAM5[Resource::*]:'), - }), - }) - ); + expectMessages(messages, { + notContaining: [ + 'AwsSolutions-IAM5[Action::s3:*]:', + 'AwsSolutions-IAM5[Resource::*]:', + ], + }); }); test('Test granular suppression when suppressed finely', () => { const stack = new Stack(); @@ -145,20 +133,11 @@ describe('Rule suppression system', () => { true ); const { messages } = SynthUtils.synthesize(stack); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-IAM5[Action::s3:*]:'), - }), - }) - ); - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-IAM5[Resource::*]:'), - }), - }) - ); + + expectMessages(messages, { + notContaining: ['AwsSolutions-IAM5[Action::s3:*]:'], + containing: ['AwsSolutions-IAM5[Resource::*]:'], + }); }); test('Test granular suppression when suppressed finely with a RegExp', () => { const stack = new Stack(); @@ -182,22 +161,10 @@ describe('Rule suppression system', () => { true ); const { messages } = SynthUtils.synthesize(stack); - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-IAM5[Action::s3:*]:'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'AwsSolutions-IAM5[Resource::some-arn:mybucket/*]:' - ), - }), - }) - ); + expectMessages(messages, { + containing: ['AwsSolutions-IAM5[Action::s3:*]:'], + notContaining: ['AwsSolutions-IAM5[Resource::some-arn:mybucket/*]:'], + }); }); test('Test rule suppression does not overrite aws:cdk:path', () => { const stack = new Stack(); @@ -285,22 +252,12 @@ describe('Rule suppression system', () => { }); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-EC23:'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'AwsSolutions-EC2SecurityGroupDescription:' - ), - }), - }) - ); + expectMessages(messages, { + notContaining: [ + 'AwsSolutions-EC23:', + 'AwsSolutions-EC2SecurityGroupDescription:', + ], + }); }); test('Test multi resource suppression', () => { const stack = new Stack(); @@ -401,22 +358,12 @@ describe('Rule suppression system', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-EC23:'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'AwsSolutions-EC2SecurityGroupDescription:' - ), - }), - }) - ); + expectMessages(messages, { + notContaining: [ + 'AwsSolutions-EC23:', + 'AwsSolutions-EC2SecurityGroupDescription:', + ], + }); }); test('addResourceSuppressions function does not override previous suppressions on a CfnResource based Construct', () => { const stack = new Stack(); @@ -473,22 +420,12 @@ describe('Rule suppression system', () => { }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-EC23:'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'AwsSolutions-EC2SecurityGroupDescription:' - ), - }), - }) - ); + expectMessages(messages, { + notContaining: [ + 'AwsSolutions-EC23:', + 'AwsSolutions-EC2SecurityGroupDescription:', + ], + }); }); test('Test suppressions with addResourceSuppressionsByPath on multiple resources', () => { const stack = new Stack(); @@ -841,22 +778,12 @@ describe('Rule suppression system', () => { { id: 'AwsSolutions-EC23', reason: 'lorem ipsum' }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('AwsSolutions-EC23:'), - }), - }) - ); - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'AwsSolutions-EC2SecurityGroupDescription:' - ), - }), - }) - ); + expectMessages(messages, { + notContaining: [ + 'AwsSolutions-EC23:', + 'AwsSolutions-EC2SecurityGroupDescription:', + ], + }); }); test('Resource suppressions change metadata on L1 Constructs', () => { const stack = new Stack(); @@ -901,13 +828,9 @@ describe('Rule suppression system', () => { { id: 'AwsSolutions-EC23', reason: 'lorem ipsum' }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('CdkNagSuppression: AwsSolutions-EC23'), - }), - }) - ); + expectMessages(messages, { + containing: ['CdkNagSuppression: AwsSolutions-EC23'], + }); }); test('suppressed rule logging disabled', () => { const stack = new Stack(); @@ -921,13 +844,9 @@ describe('Rule suppression system', () => { { id: 'AwsSolutions-EC23', reason: 'lorem ipsum' }, ]); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('CdkNagSuppression: AwsSolutions-EC23'), - }), - }) - ); + expectMessages(messages, { + notContaining: ['CdkNagSuppression: AwsSolutions-EC23'], + }); }); test('Test path miss', () => { const stack = new Stack(); @@ -953,13 +872,9 @@ describe('Rule explanations', () => { }); test.addIngressRule(Peer.anyIpv4(), Port.allTraffic()); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Large port ranges'), - }), - }) - ); + expectMessages(messages, { + notContaining: ['Large port ranges'], + }); }); test('Test no explanation', () => { const stack = new Stack(); @@ -969,13 +884,9 @@ describe('Rule explanations', () => { }); test.addIngressRule(Peer.anyIpv4(), Port.allTraffic()); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Large port ranges'), - }), - }) - ); + expectMessages(messages, { + notContaining: ['Large port ranges'], + }); }); test('Test explanation', () => { const stack = new Stack(); @@ -985,28 +896,24 @@ describe('Rule explanations', () => { }); test.addIngressRule(Peer.anyIpv4(), Port.allTraffic()); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('Large port ranges'), - }), - }) - ); + expectMessages(messages, { + containing: ['Large port ranges'], + }); }); }); describe('Rule exception handling', () => { const ERROR_MESSAGE = 'oops!'; class BadPack extends NagPack { - constructor(props?: NagPackProps) { + constructor(props?: NagPackProps, packName?: string) { super(props); - this.packName = 'Bad.Pack'; + this.packName = packName ?? 'Bad.Pack'; } public visit(node: IConstruct): void { if (node instanceof CfnResource) { this.applyRule({ ruleSuffixOverride: 'BadRule', - info: 'This is a imporperly made rule.', + info: 'This is an improperly made rule.', explanation: 'This will throw an error', level: NagMessageLevel.ERROR, rule: function (node2: CfnResource): NagRuleCompliance { @@ -1025,26 +932,18 @@ describe('Rule exception handling', () => { Aspects.of(stack).add(new BadPack()); new CfnBucket(stack, 'rBucket'); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('CdkNagValidationFailure:'), - }), - }) - ); + expectMessages(messages, { + containing: ['CdkNagValidationFailure[Bad.Pack-BadRule]:'], + }); }); test('Error properly handles verbose logging', () => { const stack = new Stack(); Aspects.of(stack).add(new BadPack({ verbose: true })); new CfnBucket(stack, 'rBucket'); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining(ERROR_MESSAGE), - }), - }) - ); + expectMessages(messages, { + containing: [ERROR_MESSAGE], + }); }); test('Error can be suppressed', () => { const stack = new Stack(); @@ -1055,13 +954,138 @@ describe('Rule exception handling', () => { ], }); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).not.toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining('CdkNagValidationFailure:'), - }), - }) + expectMessages(messages, { + notContaining: ['CdkNagValidationFailure[Bad.Pack-BadRule]:'], + }); + }); + test('Specific errors can be suppressed', () => { + const stack = new Stack(); + Aspects.of(stack).add(new BadPack({ verbose: true }, 'Bad.Pack.1')); + Aspects.of(stack).add(new BadPack({ verbose: true }, 'Bad.Pack.2')); + new CfnBucket(stack, 'rBucket').addMetadata('cdk_nag', { + rules_to_suppress: [ + { + id: 'CdkNagValidationFailure', + reason: 'at least 10 characters', + appliesTo: ['Bad.Pack.1-BadRule'], + }, + ], + }); + const messages = SynthUtils.synthesize(stack).messages; + expectMessages(messages, { + notContaining: ['CdkNagValidationFailure[Bad.Pack.1-BadRule]:'], + containing: ['CdkNagValidationFailure[Bad.Pack.2-BadRule]:'], + }); + }); + test('Suppressing a rule suppresses errors for the rule', () => { + const stack = new Stack(); + Aspects.of(stack).add(new BadPack({ verbose: true }, 'Bad.Pack.1')); + Aspects.of(stack).add(new BadPack({ verbose: true }, 'Bad.Pack.2')); + new CfnBucket(stack, 'rBucket').addMetadata('cdk_nag', { + rules_to_suppress: [ + { + id: 'Bad.Pack.1-BadRule', + reason: 'at least 10 characters', + }, + ], + }); + const messages = SynthUtils.synthesize(stack).messages; + expectMessages(messages, { + notContaining: ['CdkNagValidationFailure[Bad.Pack.1-BadRule]:'], + containing: ['CdkNagValidationFailure[Bad.Pack.2-BadRule]:'], + }); + }); + test.each([ + [ + 'NagPack level suppression', + { + id: 'Bad.Pack.1-BadRule', + reason: 'at least 10 characters', + }, + ], + [ + 'CdkNagValidationFailure suppression', + { + id: 'CdkNagValidationFailure', + reason: 'at least 10 characters', + }, + ], + [ + 'CdkNagValidationFailure granular suppression', + { + id: 'CdkNagValidationFailure', + reason: 'at least 10 characters', + appliesTo: ['Bad.Pack.1-BadRule'], + }, + ], + ])( + 'Suppression ignore conditions apply to exceptions with %s', + (_testName, suppressionRule) => { + const stack = new Stack(); + Aspects.of(stack).add( + new BadPack( + { + suppressionIgnoreCondition: new SuppressionIgnoreAlways('IGNORED.'), + }, + 'Bad.Pack.1' + ) + ); + new CfnBucket(stack, 'rBucket').addMetadata('cdk_nag', { + rules_to_suppress: [suppressionRule], + }); + const messages = SynthUtils.synthesize(stack).messages; + expectMessages(messages, { + containing: [ + 'CdkNagValidationFailure[Bad.Pack.1-BadRule]:', + 'The suppression for Bad.Pack.1-BadRule was ignored', + ], + }); + } + ); + test('Suppressing CdkNagValidationFailure still suppresses other failures when one has a suppression ignore', () => { + const stack = new Stack(); + Aspects.of(stack).add( + new BadPack( + { + suppressionIgnoreCondition: new SuppressionIgnoreAlways('IGNORED.'), + }, + 'Bad.Pack.1' + ) ); + Aspects.of(stack).add(new BadPack({ verbose: true }, 'Bad.Pack.2')); + new CfnBucket(stack, 'rBucket').addMetadata('cdk_nag', { + rules_to_suppress: [ + { + id: 'CdkNagValidationFailure', + reason: 'at least 10 characters', + }, + ], + }); + const messages = SynthUtils.synthesize(stack).messages; + expectMessages(messages, { + containing: [ + 'CdkNagValidationFailure[Bad.Pack.1-BadRule]:', + 'The suppression for Bad.Pack.1-BadRule was ignored', + ], + notContaining: ['CdkNagValidationFailure[Bad.Pack.2-BadRule]:'], + }); + }); + test('Granularly suppressing a rule does not suppress errors for the rule', () => { + const stack = new Stack(); + Aspects.of(stack).add(new BadPack({ verbose: true }, 'Bad.Pack.1')); + new CfnBucket(stack, 'rBucket').addMetadata('cdk_nag', { + rules_to_suppress: [ + { + id: 'Bad.Pack.1-BadRule', + reason: 'at least 10 characters', + appliesTo: ['foo'], + }, + ], + }); + const messages = SynthUtils.synthesize(stack).messages; + expectMessages(messages, { + containing: ['CdkNagValidationFailure[Bad.Pack.1-BadRule]:'], + }); }); test('Suppressed validation error is logged with suppressed rule logging', () => { const stack = new Stack(); @@ -1072,15 +1096,32 @@ describe('Rule exception handling', () => { ], }); const messages = SynthUtils.synthesize(stack).messages; - expect(messages).toContainEqual( - expect.objectContaining({ - entry: expect.objectContaining({ - data: expect.stringContaining( - 'CdkNagSuppression: CdkNagValidationFailure' - ), - }), - }) - ); + expectMessages(messages, { + containing: [ + 'CdkNagSuppression[Bad.Pack-BadRule]: CdkNagValidationFailure', + ], + }); + }); + test('Suppressed validation error is logged with specifically suppressed rule logging', () => { + const stack = new Stack(); + Aspects.of(stack).add(new BadPack({ logIgnores: true }, 'Bad.Pack.1')); + Aspects.of(stack).add(new BadPack({}, 'Bad.Pack.2')); + new CfnBucket(stack, 'rBucket').addMetadata('cdk_nag', { + rules_to_suppress: [ + { + id: 'CdkNagValidationFailure', + reason: 'at least 10 characters', + appliesTo: ['Bad.Pack.1-BadRule'], + }, + ], + }); + const messages = SynthUtils.synthesize(stack).messages; + expectMessages(messages, { + containing: [ + 'CdkNagSuppression[Bad.Pack.1-BadRule]: CdkNagValidationFailure', + 'CdkNagValidationFailure[Bad.Pack.2-BadRule]:', + ], + }); }); test('Encoded Intrinsic function with resolveIfPrimitive error handling', () => { const stack = new Stack(); diff --git a/test/Loggers.test.ts b/test/Loggers.test.ts index d84df802f8..954c0ae274 100644 --- a/test/Loggers.test.ts +++ b/test/Loggers.test.ts @@ -248,7 +248,7 @@ describe('NagReportLogger', () => { ]); app.synth(); const expectedOutput = [ - '"Test-Non-Compliant","Stack1/rResource","UNKNOWN","N/A","Error","foo."', + '"Test-Non-Compliant","Stack1/rResource","Suppressed","lorem ipsum","Error","foo."', '"Test-Compliant","Stack1/rResource","UNKNOWN","N/A","Error","foo."', '"Test-N/A","Stack1/rResource","UNKNOWN","N/A","Error","foo."', ]; @@ -390,6 +390,40 @@ describe('NagReportLogger', () => { ).toEqual(expect.arrayContaining(expectedOutput)); }); test('Error values are written properly', () => { + const stack = new Stack(app, 'Stack1'); + new CfnResource(stack, 'rResource', { type: 'Error' }); + app.synth(); + const expectedOutput: NagReportLine[] = [ + { + ruleId: 'Test-Compliant', + resourceId: 'Stack1/rResource', + compliance: 'UNKNOWN', + exceptionReason: 'N/A', + ruleLevel: 'Error', + ruleInfo: 'foo.', + }, + { + ruleId: 'Test-Non-Compliant', + resourceId: 'Stack1/rResource', + compliance: 'UNKNOWN', + exceptionReason: 'N/A', + ruleLevel: 'Error', + ruleInfo: 'foo.', + }, + { + ruleId: 'Test-N/A', + resourceId: 'Stack1/rResource', + compliance: 'UNKNOWN', + exceptionReason: 'N/A', + ruleLevel: 'Error', + ruleInfo: 'foo.', + }, + ]; + expect( + getReportLines(reportLogger.getFormatStacks(NagReportFormat.JSON)[0]) + ).toEqual(expect.arrayContaining(expectedOutput)); + }); + test('Error values are suppressed properly', () => { const stack = new Stack(app, 'Stack1'); const resource = new CfnResource(stack, 'rResource', { type: 'Error' }); NagSuppressions.addResourceSuppressions(resource, [ @@ -411,8 +445,8 @@ describe('NagReportLogger', () => { { ruleId: 'Test-Non-Compliant', resourceId: 'Stack1/rResource', - compliance: 'UNKNOWN', - exceptionReason: 'N/A', + compliance: 'Suppressed', + exceptionReason: 'lorem ipsum', ruleLevel: 'Error', ruleInfo: 'foo.', }, diff --git a/test/test-utils.ts b/test/test-utils.ts new file mode 100644 index 0000000000..030f2ee52b --- /dev/null +++ b/test/test-utils.ts @@ -0,0 +1,41 @@ +/* +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: Apache-2.0 +*/ +import { SynthesisMessage } from 'aws-cdk-lib/cx-api'; + +interface ExpectMessageConditions { + readonly containing?: string[]; + readonly notContaining?: string[]; + readonly length?: number; +} +export function expectMessages( + messages: SynthesisMessage[], + conditions: ExpectMessageConditions +) { + if (conditions.containing) { + for (const condition of conditions.containing) { + expect(messages).toContainEqual( + expect.objectContaining({ + entry: expect.objectContaining({ + data: expect.stringContaining(condition), + }), + }) + ); + } + } + if (conditions.notContaining) { + for (const condition of conditions.notContaining) { + expect(messages).not.toContainEqual( + expect.objectContaining({ + entry: expect.objectContaining({ + data: expect.stringContaining(condition), + }), + }) + ); + } + } + if (conditions.length) { + expect(messages.length).toEqual(conditions.length); + } +}