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(cdk): aspect framework and tag implementation #1451

Merged
merged 41 commits into from
Feb 6, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
9a8c82b
feat(cdk): Add support for Aspects (#1451)
moofish32 Oct 18, 2018
419a701
fixing public access to resource properties
moofish32 Jan 14, 2019
0a0d010
fixing dynamo tag implementation
moofish32 Jan 14, 2019
0109956
refactoring to add in a TestStack for easy unit testing when aspects …
moofish32 Jan 16, 2019
2188764
adding documentation to classes
moofish32 Jan 16, 2019
ce95e64
cleaning up some extraneous comments
moofish32 Jan 16, 2019
f3a7e37
adding tag-manager unit tests
moofish32 Jan 16, 2019
6cc19db
convert dynamo test to use expect
moofish32 Jan 16, 2019
67a738b
test stack cleanup for kms
moofish32 Jan 16, 2019
068726a
fixing dynamo typos in tests
moofish32 Jan 16, 2019
6389574
refactoring for naming of visitTree and PR comments
moofish32 Jan 18, 2019
12be7ad
moving aspects into prepareTree()
moofish32 Jan 18, 2019
257a15f
refactor aspect control to construct
moofish32 Jan 19, 2019
522e08c
adding multiple visit aspects
moofish32 Jan 19, 2019
2467a5e
reverting TestStack
moofish32 Jan 19, 2019
fdae887
linter fixes
moofish32 Jan 19, 2019
7409e5f
adding an aspect readme
moofish32 Jan 21, 2019
4ffbd55
refactoring to add tag priorities and minor readme updates, plus name…
moofish32 Jan 27, 2019
46e2f70
Adding a Tag Readme section more focused on end user tagging and less…
moofish32 Jan 27, 2019
69401f1
removing multiple aspects
moofish32 Jan 27, 2019
5514650
visitor pattern link correction
moofish32 Jan 27, 2019
25b4427
README updates and small comment improvements
moofish32 Feb 2, 2019
02fb644
feat(cdk): aspects and tagging #1451
moofish32 Feb 2, 2019
3ddfe99
Merge branch 'master' into f-tags-aspects
Feb 4, 2019
12cfe19
adding cdk.json for tag example
moofish32 Feb 4, 2019
dcc5303
feat(aws-ecs): add support for Event Targets (#1571)
rix0rrr Feb 4, 2019
2cbd2c0
feat(app): add source map support to TS app template (#1581)
otterley Feb 4, 2019
e3f5ad3
fix(apig): Move 'selectionPattern` to `integrationResponses` (#1636)
RomainMuller Feb 4, 2019
f706c3d
feat(core): generalization of dependencies (#1583)
rix0rrr Feb 4, 2019
fec19b6
v0.23.0 (#1665)
RomainMuller Feb 4, 2019
3bc66f2
removing tag aspect props and just using tag props
moofish32 Feb 4, 2019
aeb24f1
feat(codedeploy) lambda application and deployment groups (#1628)
Feb 4, 2019
db75215
chore(docs): move developer guide to docs.aws.amazon.com (#1470)
Doug-AWS Feb 4, 2019
fc16433
feat(aws-s3): add option to specify block public access settings (#1664)
jogold Feb 4, 2019
a91a4f1
refactor to eliminate TagAspectProps, requires TagManager to know Res…
moofish32 Feb 5, 2019
5970c0b
feat(cdk): aspect framework and tag implementation #1451
moofish32 Feb 5, 2019
395a83f
Merge branch 'master' into f-tags-aspects
moofish32 Feb 5, 2019
bafff0f
minor fix to the example
moofish32 Feb 5, 2019
fd94030
fix test in tag-manager to add resource type string for clarity
moofish32 Feb 5, 2019
3bfa467
Merge branch 'master' into f-tags-aspects
moofish32 Feb 5, 2019
d0fd751
updating integ test that now tags the lambda function
moofish32 Feb 6, 2019
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
34 changes: 8 additions & 26 deletions packages/@aws-cdk/cdk/lib/aspects/tag-aspect.ts
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
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
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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an important behavior to note. I could make this more complicated and solve merges. However, I think it's easy to move to the new Tag and keeps the code simple.

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
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, '');
moofish32 marked this conversation as resolved.
Show resolved Hide resolved
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
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