Skip to content

Commit

Permalink
refactor to eliminate TagAspectProps, requires TagManager to know Res…
Browse files Browse the repository at this point in the history 
…ource Type
  • Loading branch information
@moofish32
moofish32 committed Feb 5, 2019
1 parent fc16433 commit a91a4f1
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 98 deletions.
34 changes: 8 additions & 26 deletions packages/@aws-cdk/cdk/lib/aspects/tag-aspect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@ export abstract class TagBase implements IAspect {
*/
public readonly key: string;

private readonly includeResourceTypes: string[];
private readonly excludeResourceTypes: string[];
protected readonly props: TagProps;

constructor(key: string, props: TagProps = {}) {
this.key = key;
this.includeResourceTypes = props.includeResourceTypes || [];
this.excludeResourceTypes = props.excludeResourceTypes || [];
this.props = props;
}

public visit(construct: IConstruct): void {
Expand All @@ -28,14 +26,6 @@ export abstract class TagBase implements IAspect {
}
const resource = construct as Resource;
if (Resource.isTaggable(resource)) {
if (this.excludeResourceTypes.length !== 0 &&
this.excludeResourceTypes.indexOf(resource.resourceType) !== -1) {
return;
}
if (this.includeResourceTypes.length !== 0 &&
this.includeResourceTypes.indexOf(resource.resourceType) === -1) {
return;
}
this.applyTag(resource);
}
}
Expand All @@ -53,24 +43,18 @@ export class Tag extends TagBase {
*/
public readonly value: string;

private readonly applyToLaunchedInstances: boolean;
private readonly priority: number;

constructor(key: string, value: string, props: TagProps = {}) {
super(key, {...props});
this.applyToLaunchedInstances = props.applyToLaunchedInstances !== false;
this.priority = props.priority === undefined ? 0 : props.priority;
super(key, props);
this.props.applyToLaunchedInstances = props.applyToLaunchedInstances !== false;
this.props.priority = props.priority === undefined ? 0 : props.priority;
if (value === undefined) {
throw new Error('Tag must have a value');
}
this.value = value;
}

protected applyTag(resource: ITaggable) {
resource.tags.setTag(this.key, this.value!, {
applyToLaunchedInstances: this.applyToLaunchedInstances,
priority: this.priority,
});
resource.tags.setTag(this.key, this.value!, this.props);
}
}

Expand All @@ -79,15 +63,13 @@ export class Tag extends TagBase {
*/
export class RemoveTag extends TagBase {

private readonly priority: number;

constructor(key: string, props: TagProps = {}) {
super(key, props);
this.priority = props.priority === undefined ? 1 : props.priority;
this.props.priority = props.priority === undefined ? 1 : props.priority;
}

protected applyTag(resource: ITaggable): void {
resource.tags.removeTag(this.key, this.priority);
resource.tags.removeTag(this.key, this.props);
return;
}
}
45 changes: 38 additions & 7 deletions packages/@aws-cdk/cdk/lib/core/tag-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,32 @@ export interface TagProps {
*/
applyToLaunchedInstances?: boolean;

/**
* An array of Resource Types that will not receive this tag
*
* An empty array will allow this tag to be applied to all resources. A
* non-empty array will apply this tag only if the Resource type is not in
* this array.
* @default []
*/
excludeResourceTypes?: string[];

/**
* An array of Resource Types that will receive this tag
*
* An empty array will match any Resource. A non-empty array will apply this
* tag only to Resource types that are included in this array.
* @default []
*/
includeResourceTypes?: string[];

/**
* Higher or equal priority tags will take precedence
*
* Setting priority will enable the user to control tags when they need to not
* follow the default precedence pattern of last applied and closest to the
* construct in the tree.
* @default 0 for Tag 1 for Remove Tag
* @default 0 for Tag 1 for RemoveTag
*/
priority?: number;
}
Expand All @@ -34,7 +53,7 @@ export class TagManager {

private readonly removedTags: {[key: string]: number} = {};

constructor(private readonly tagType: TagType) { }
constructor(private readonly tagType: TagType, private readonly resourceTypeName: string) { }

/**
* Adds the specified tag to the array of tags
Expand All @@ -45,9 +64,8 @@ export class TagManager {
*/
public setTag(key: string, value: string, props?: TagProps): void {
const tagProps: TagProps = props || {};
tagProps.priority = tagProps.priority === undefined ? 0 : tagProps.priority;

if (!this.hasHigherPriority(key, tagProps.priority)) {
if (!this.canApplyTag(key, tagProps)) {
// tag is blocked by a remove
return;
}
Expand All @@ -62,8 +80,10 @@ export class TagManager {
*
* @param key The key of the tag to remove
*/
public removeTag(key: string, priority: number = 0): void {
if (!this.hasHigherPriority(key, priority)) {
public removeTag(key: string, props?: TagProps): void {
const tagProps = props || {};
const priority = tagProps.priority === undefined ? 0 : tagProps.priority;
if (!this.canApplyTag(key, tagProps)) {
// tag is blocked by a remove
return;
}
Expand Down Expand Up @@ -108,7 +128,18 @@ export class TagManager {
}
}

private hasHigherPriority(key: string, priority: number): boolean {
private canApplyTag(key: string, props: TagProps): boolean {
const include = props.includeResourceTypes || [];
const exclude = props.excludeResourceTypes || [];
const priority = props.priority === undefined ? 0 : props.priority;
if (exclude.length !== 0 &&
exclude.indexOf(this.resourceTypeName) !== -1) {
return false;
}
if (include.length !== 0 &&
include.indexOf(this.resourceTypeName) === -1) {
return false;
}
if (this.tags[key]) {
if (this.tags[key].props.priority !== undefined) {
return priority >= this.tags[key].props.priority!;
Expand Down
53 changes: 3 additions & 50 deletions packages/@aws-cdk/cdk/test/aspects/test.tag-aspect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@ import { Test } from 'nodeunit';
import { RemoveTag, Resource, Stack, Tag, TagManager, TagType } from '../../lib';

class TaggableResource extends Resource {
public readonly tags = new TagManager(TagType.Standard);
public readonly tags = new TagManager(TagType.Standard, 'AWS::Fake::Resource');
public testProperties() {
return this.properties;
}
}

class AsgTaggableResource extends TaggableResource {
public readonly tags = new TagManager(TagType.AutoScalingGroup);
public readonly tags = new TagManager(TagType.AutoScalingGroup, 'AWS::Fake::Resource');
}

class MapTaggableResource extends TaggableResource {
public readonly tags = new TagManager(TagType.Map);
public readonly tags = new TagManager(TagType.Map, 'AWS::Fake::Resource');
}

export = {
Expand Down Expand Up @@ -100,30 +100,6 @@ export = {
test.deepEqual(res.tags.renderTags(), [{key: 'foo', value: 'bar'}]);
test.done();
},
'include restricts tag application to resources types in the list'(test: Test) {
const root = new Stack();
const res = new TaggableResource(root, 'FakeResource', {
type: 'AWS::Fake::Thing',
});
const res2 = new TaggableResource(res, 'FakeResource', {
type: 'AWS::Fake::Thing',
});
const asg = new AsgTaggableResource(res, 'AsgFakeResource', {
type: 'AWS::Fake::Asg',
});

const map = new MapTaggableResource(res, 'MapFakeResource', {
type: 'AWS::Fake::Map',
});
res.apply(new Tag('foo', 'bar', {includeResourceTypes: ['AWS::Fake::Asg']}));
root.node.prepareTree();
test.deepEqual(res.tags.renderTags(), undefined);
test.deepEqual(map.tags.renderTags(), undefined);
test.deepEqual(res2.tags.renderTags(), undefined);
test.deepEqual(asg.tags.renderTags(), [{key: 'foo', value: 'bar', propagateAtLaunch: true}]);
test.deepEqual(map.testProperties().tags, undefined);
test.done();
},
'removeTag Aspects by default will override child Tag Aspects'(test: Test) {
const root = new Stack();
const res = new TaggableResource(root, 'FakeResource', {
Expand Down Expand Up @@ -154,29 +130,6 @@ export = {
test.deepEqual(res2.tags.renderTags(), [{key: 'key', value: 'value'}]);
test.done();
},
'exclude prevents tag application to resource types in the list'(test: Test) {
const root = new Stack();
const res = new TaggableResource(root, 'FakeResource', {
type: 'AWS::Fake::Thing',
});
const res2 = new TaggableResource(res, 'FakeResource', {
type: 'AWS::Fake::Thing',
});
const asg = new AsgTaggableResource(res, 'AsgFakeResource', {
type: 'AWS::Fake::Asg',
});

const map = new MapTaggableResource(res, 'MapFakeResource', {
type: 'AWS::Fake::Map',
});
res.apply(new Tag('foo', 'bar', {excludeResourceTypes: ['AWS::Fake::Asg']}));
root.node.prepareTree();
test.deepEqual(res.tags.renderTags(), [{key: 'foo', value: 'bar'}]);
test.deepEqual(res2.tags.renderTags(), [{key: 'foo', value: 'bar'}]);
test.deepEqual(asg.tags.renderTags(), undefined);
test.deepEqual(map.tags.renderTags(), {foo: 'bar'});
test.done();
},
'Aspects are mutually exclusive with tags created by L1 Constructor'(test: Test) {
const root = new Stack();
const aspectBranch = new TaggableResource(root, 'FakeBranchA', {
Expand Down
38 changes: 26 additions & 12 deletions packages/@aws-cdk/cdk/test/core/test.tag-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,34 @@ import { TagManager } from '../../lib/core/tag-manager';

export = {
'#setTag() supports setting a tag regardless of Type'(test: Test) {
const notTaggable = new TagManager(TagType.NotTaggable);
const notTaggable = new TagManager(TagType.NotTaggable, '');
notTaggable.setTag('key', 'value');
test.deepEqual(notTaggable.renderTags(), undefined);
test.done();
},
'when a tag does not exist': {
'#removeTag() does not throw an error'(test: Test) {
const mgr = new TagManager(TagType.Standard);
const mgr = new TagManager(TagType.Standard, '');
test.doesNotThrow(() => (mgr.removeTag('dne')));
test.done();
},
'#setTag() creates the tag'(test: Test) {
const mgr = new TagManager(TagType.Standard);
const mgr = new TagManager(TagType.Standard, '');
mgr.setTag('dne', 'notanymore');
test.deepEqual(mgr.renderTags(), [{key: 'dne', value: 'notanymore'}]);
test.done();
}
},
'when a tag does exist': {
'#removeTag() deletes the tag'(test: Test) {
const mgr = new TagManager(TagType.Standard);
const mgr = new TagManager(TagType.Standard, '');
mgr.setTag('dne', 'notanymore');
mgr.removeTag('dne');
test.deepEqual(mgr.renderTags(), undefined);
test.done();
},
'#setTag() overwrites the tag'(test: Test) {
const mgr = new TagManager(TagType.Standard);
const mgr = new TagManager(TagType.Standard, '');
mgr.setTag('dne', 'notanymore');
mgr.setTag('dne', 'iwin');
test.deepEqual(mgr.renderTags(), [{key: 'dne', value: 'iwin'}]);
Expand All @@ -40,16 +40,16 @@ export = {
},
'when there are no tags': {
'#renderTags() returns undefined'(test: Test) {
const mgr = new TagManager(TagType.Standard);
const mgr = new TagManager(TagType.Standard, '');
test.deepEqual(mgr.renderTags(), undefined );
test.done();
},
},
'#renderTags() handles standard, map, and ASG tag formats'(test: Test) {
const tagged: TagManager[] = [];
const standard = new TagManager(TagType.Standard);
const asg = new TagManager(TagType.AutoScalingGroup);
const mapper = new TagManager(TagType.Map);
const standard = new TagManager(TagType.Standard, '');
const asg = new TagManager(TagType.AutoScalingGroup, '');
const mapper = new TagManager(TagType.Map, '');
tagged.push(standard);
tagged.push(asg);
tagged.push(mapper);
Expand All @@ -72,19 +72,33 @@ export = {
test.done();
},
'tags with higher or equal priority always take precedence'(test: Test) {
const mgr = new TagManager(TagType.Standard);
const mgr = new TagManager(TagType.Standard, '');
mgr.setTag('key', 'myVal', {
priority: 2,
});
mgr.setTag('key', 'newVal', {
priority: 1,
});
mgr.removeTag('key', 1);
mgr.removeTag('key', {priority: 1});
test.deepEqual(mgr.renderTags(), [
{key: 'key', value: 'myVal'},
]);
mgr.removeTag('key', 2);
mgr.removeTag('key', {priority: 2});
test.deepEqual(mgr.renderTags(), undefined);
test.done();
},
'excludeResourceTypes only tags resources that do not match'(test: Test) {
const mgr = new TagManager(TagType.Standard, 'AWS::Fake::Resource');
mgr.setTag('key', 'value', {excludeResourceTypes: ['AWS::Fake::Resource']});
mgr.setTag('sticky', 'value', {excludeResourceTypes: ['AWS::Wrong::Resource']});
test.deepEqual(mgr.renderTags(), [{key: 'sticky', value: 'value'}]);
test.done();
},
'includeResourceTypes only tags resources that match'(test: Test) {
const mgr = new TagManager(TagType.Standard, 'AWS::Fake::Resource');
mgr.setTag('key', 'value', {includeResourceTypes: ['AWS::Fake::Resource']});
mgr.setTag('sticky', 'value', {includeResourceTypes: ['AWS::Wrong::Resource']});
test.deepEqual(mgr.renderTags(), [{key: 'key', value: 'value'}]);
test.done();
}
};
7 changes: 4 additions & 3 deletions tools/cfn2ts/lib/codegen.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,11 @@ export default class CodeGenerator {
// Static inspectors.
//

const resourceTypeName = `${JSON.stringify(resourceName.specName!.fqn)}`;
this.code.line('/**');
this.code.line(` * The CloudFormation resource type name for this resource class.`);
this.code.line(' */');
this.code.line(`public static readonly resourceTypeName = ${JSON.stringify(resourceName.specName!.fqn)};`);
this.code.line(`public static readonly resourceTypeName = ${resourceTypeName};`);

if (spec.RequiredTransform) {
this.code.line('/**');
Expand Down Expand Up @@ -242,10 +243,10 @@ export default class CodeGenerator {
this.code.line(' *');
this.code.line(' * Tags should be managed either passing them as properties during');
this.code.line(' * initiation or by calling methods on this object. If both techniques are');
this.code.line(' * used only the tags from the TagManager will be used. ``TagAspects``');
this.code.line(' * used only the tags from the TagManager will be used. ``Tag`` (aspect)');
this.code.line(' * will use the manager.');
this.code.line(' */');
this.code.line(`public readonly tags = new ${TAG_MANAGER}(${tagEnum});`);
this.code.line(`public readonly tags = new ${TAG_MANAGER}(${tagEnum}, ${resourceTypeName});`);
}

//
Expand Down

0 comments on commit a91a4f1

Please sign in to comment.