Skip to content

Commit

Permalink
fix(assertions): hasResourceProperties is incompatible with `Match.…
Browse files Browse the repository at this point in the history
…not` and `Match.absent` (aws#16678)

Fixes aws#16626.

This PR modifies `hasResourceProperties()` so that it accounts for the special case where `Properties` does not exist on the template. The following assertions previously behaved incorrectly when `Properties` were not in the template and are now fixed:

```ts
template.fromStack(stack); // some template with no `Properties`.

// assert that `Properties` does not exist in the template. Returns true.
template.hasResourceProperties('Foo::Bar', Match.absent());

// assert that `baz` is not a `Property` of 'Foo::Bar'. Returns true.
template.hasResourceProperties('Foo::Bar', {
   baz: Match.absent(),
};

// assert that `baz` does not have a value of `qux` in the `Properties` object. Returns true.
template.hasResourceProperties('Foo::Bar', Match.not({baz: 'qux'});
```

It also moves `AbsentMatch` into the `private` folder so that it can be exposed internally as a special case for `hasResourceProperties()`.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc committed Oct 6, 2021
1 parent d6a4de9 commit 6f0a507
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 22 deletions.
15 changes: 1 addition & 14 deletions packages/@aws-cdk/assertions/lib/match.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Matcher, MatchResult } from './matcher';
import { AbsentMatch } from './private/matchers/absent';
import { getType } from './private/type';

/**
Expand Down Expand Up @@ -329,17 +330,3 @@ class AnyMatch extends Matcher {
return result;
}
}

class AbsentMatch extends Matcher {
constructor(public readonly name: string) {
super();
}

public test(actual: any): MatchResult {
const result = new MatchResult(actual);
if (actual !== undefined) {
result.push(this, [], `Received ${actual}, but key should be absent`);
}
return result;
}
}
15 changes: 15 additions & 0 deletions packages/@aws-cdk/assertions/lib/private/matchers/absent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { Matcher, MatchResult } from '../../matcher';

export class AbsentMatch extends Matcher {
constructor(public readonly name: string) {
super();
}

public test(actual: any): MatchResult {
const result = new MatchResult(actual);
if (actual !== undefined) {
result.push(this, [], `Received ${actual}, but key should be absent`);
}
return result;
}
}
29 changes: 28 additions & 1 deletion packages/@aws-cdk/assertions/lib/private/resources.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Match, Matcher } from '..';
import { AbsentMatch } from './matchers/absent';
import { formatFailure, matchSection } from './section';
import { Resource, Template } from './template';

Expand All @@ -15,7 +17,6 @@ export function findResources(template: Template, type: string, props: any = {})
export function hasResource(template: Template, type: string, props: any): string | void {
const section = template.Resources;
const result = matchSection(filterType(section, type), props);

if (result.match) {
return;
}
Expand All @@ -30,13 +31,39 @@ export function hasResource(template: Template, type: string, props: any): strin
].join('\n');
}

export function hasResourceProperties(template: Template, type: string, props: any): string | void {
// amended needs to be a deep copy to avoid modifying the template.
let amended = JSON.parse(JSON.stringify(template));

// special case to exclude AbsentMatch because adding an empty Properties object will affect its evaluation.
if (!Matcher.isMatcher(props) || !(props instanceof AbsentMatch)) {
amended = addEmptyProperties(amended);
}

return hasResource(amended, type, Match.objectLike({
Properties: props,
}));
}

export function countResources(template: Template, type: string): number {
const section = template.Resources;
const types = filterType(section, type);

return Object.entries(types).length;
}

function addEmptyProperties(template: Template): Template {
let section = template.Resources;

Object.keys(section).map((key) => {
if (!section[key].hasOwnProperty('Properties')) {
section[key].Properties = {};
}
});

return template;
}

function filterType(section: { [key: string]: Resource }, type: string): { [key: string]: Resource } {
return Object.entries(section ?? {})
.filter(([_, v]) => v.Type === type)
Expand Down
9 changes: 5 additions & 4 deletions packages/@aws-cdk/assertions/lib/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Match } from './match';
import { Matcher } from './matcher';
import { findMappings, hasMapping } from './private/mappings';
import { findOutputs, hasOutput } from './private/outputs';
import { countResources, findResources, hasResource } from './private/resources';
import { countResources, findResources, hasResource, hasResourceProperties } from './private/resources';
import { Template as TemplateType } from './private/template';

/**
Expand Down Expand Up @@ -74,9 +74,10 @@ export class Template {
* @param props the 'Properties' section of the resource as should be expected in the template.
*/
public hasResourceProperties(type: string, props: any): void {
this.hasResource(type, Match.objectLike({
Properties: Matcher.isMatcher(props) ? props : Match.objectLike(props),
}));
const matchError = hasResourceProperties(this.template, type, props);
if (matchError) {
throw new Error(matchError);
}
}

/**
Expand Down
39 changes: 36 additions & 3 deletions packages/@aws-cdk/assertions/test/template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,33 +270,56 @@ describe('Template', () => {
});

describe('hasResourceProperties', () => {
test('absent', () => {
test('exact match', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
properties: { baz: 'qux' },
});

const inspect = Template.fromStack(stack);
inspect.hasResourceProperties('Foo::Bar', { baz: 'qux' });

expect(() => inspect.hasResourceProperties('Foo::Bar', { baz: 'waldo' }))
.toThrow(/Expected waldo but received qux at \/Properties\/baz/);

expect(() => inspect.hasResourceProperties('Foo::Bar', { baz: 'qux', fred: 'waldo' }))
.toThrow(/Missing key at \/Properties\/fred/);
});

test('absent - with properties', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
properties: { baz: 'qux' },
});

const inspect = Template.fromStack(stack);

inspect.hasResourceProperties('Foo::Bar', {
bar: Match.absent(),
});

expect(() => inspect.hasResourceProperties('Foo::Bar', {
baz: Match.absent(),
})).toThrow(/key should be absent at \/Properties\/baz/);
});

test('absent - no properties on template', () => {
test('absent - no properties', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
});

const inspect = Template.fromStack(stack);

expect(() => inspect.hasResourceProperties('Foo::Bar', { bar: Match.absent(), baz: 'qux' }))
.toThrow(/Missing key at \/Properties\/baz/);

inspect.hasResourceProperties('Foo::Bar', Match.absent());
});

test('not', () => {
test('not - with properties', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
Expand All @@ -308,6 +331,16 @@ describe('Template', () => {
baz: 'boo',
}));
});

test('not - no properties', () => {
const stack = new Stack();
new CfnResource(stack, 'Foo', {
type: 'Foo::Bar',
});

const inspect = Template.fromStack(stack);
inspect.hasResourceProperties('Foo::Bar', Match.not({ baz: 'qux' }));
});
});

describe('getResources', () => {
Expand Down

0 comments on commit 6f0a507

Please sign in to comment.