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 9 commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import cxapi = require('@aws-cdk/cx-api');
import fc = require('fast-check');
import nodeunit = require('nodeunit');

import { countResources, expect, haveResource, isSuperObject } from '@aws-cdk/assert';
import { countResources, expect, haveResource, isSuperObject, TestStack } from '@aws-cdk/assert';
import { PipelineDeployStackAction } from '../lib/pipeline-deploy-stack-action';

interface SelfUpdatingPipeline {
Expand Down Expand Up @@ -299,7 +299,7 @@ class FakeAction extends api.Action {
}

function getTestStack(): cdk.Stack {
return new cdk.Stack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
return new TestStack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
}

function createSelfUpdatingStack(pipelineStack: cdk.Stack): SelfUpdatingPipeline {
Expand Down
9 changes: 9 additions & 0 deletions packages/@aws-cdk/assert/lib/expect.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import cdk = require('@aws-cdk/cdk');
import api = require('@aws-cdk/cx-api');
import { StackInspector } from './inspector';
import { TestStack } from './test-stack';

export function expect(stack: api.SynthesizedStack | cdk.Stack, skipValidation = false): StackInspector {
// Can't use 'instanceof' here, that breaks if we have multiple copies
Expand All @@ -18,6 +19,10 @@ export function expect(stack: api.SynthesizedStack | cdk.Stack, skipValidation =
}
}

if (isTestStack(stack)) {
stack.testInvokeAspects();
}

sstack = {
name: stack.name,
template: stack.toCloudFormation(),
Expand Down Expand Up @@ -50,3 +55,7 @@ function collectStackMetadata(root: cdk.ConstructNode): api.StackMetadata {
}
return result;
}

function isTestStack(t: any): t is TestStack {
return t.testInvokeAspects !== undefined;
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/assert/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export * from './assertion';
export * from './expect';
export * from './inspector';
export * from './test-stack';

export * from './assertions/exist';
export * from './assertions/have-resource';
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/assert/lib/test-stack.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import cdk = require('@aws-cdk/cdk');

export class TestStack extends cdk.Stack {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Is this a good idea to put in a reusable place?
  2. I started to find all the places we should use this, but I missed a few and now I'm questioning if this is too much to include here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this class is no longer needed but the test clean up seems like it was a good idea?

public testInvokeAspects() {
this.invokeAspects();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move invokeAspects behind the all new “prepareTree”?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I think that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just for clarity, I am doing:

  1. remove invokeAspects() from app.ts
  2. putting invokeAspects() inside of prepareTree()
  3. removing the TestStack because prepareTree() is public and called during expect

Is there any concern about the conditional wrapped around the block that calls prepareTree()?

    if (!skipValidation) { // any concern here - I haven't found any but just double checking
      // Do a prepare-and-validate run over the given stack
      stack.node.prepareTree();

      const errors = stack.node.validateTree();
      if (errors.length > 0) {
        throw new Error(`Stack validation failed:\n${errors.map(e => `${e.message} at: ${e.source.node.scope}`).join('\n')}`);
      }
    }

// this will be removed
    if (isTestStack(stack)) {
      stack.testInvokeAspects();
    }
// end of block to be removed

A simple thumbs up to verify is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lgtm; Curious why prepare tree is skipped if validation is skipped but that’s for a different issue (raise one?).

}
}
30 changes: 5 additions & 25 deletions packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,6 @@ export interface AutoScalingGroupProps {
*/
resourceSignalTimeoutSec?: number;

/**
* The AWS resource tags to associate with the ASG.
*/
tags?: cdk.Tags;

/**
* Default scaling cooldown for this AutoScalingGroup
*
Expand All @@ -161,7 +156,7 @@ export interface AutoScalingGroupProps {
*
* The ASG spans all availability zones.
*/
export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup, cdk.ITaggable, elb.ILoadBalancerTarget, ec2.IConnectable,
export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup, elb.ILoadBalancerTarget, ec2.IConnectable,
elbv2.IApplicationLoadBalancerTarget, elbv2.INetworkLoadBalancerTarget {
/**
* The type of OS instances of this fleet are running.
Expand All @@ -178,11 +173,6 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
*/
public readonly role: iam.Role;

/**
* Manage tags for this construct and children
*/
public readonly tags: cdk.TagManager;

/**
* Name of the AutoScalingGroup
*/
Expand All @@ -209,8 +199,9 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
});
this.connections = new ec2.Connections({ securityGroups: [this.securityGroup] });
this.securityGroups.push(this.securityGroup);
this.tags = new TagManager(this, {initialTags: props.tags});
this.tags.setTag(NAME_TAG, this.node.path, { overwrite: false });
this.apply(new cdk.Tag(NAME_TAG, this.node.path, {
applyToLaunchInstances: true,
}));

this.role = new iam.Role(this, 'InstanceRole', {
assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com')
Expand Down Expand Up @@ -255,7 +246,6 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
launchConfigurationName: launchConfig.ref,
loadBalancerNames: new cdk.Token(() => this.loadBalancerNames.length > 0 ? this.loadBalancerNames : undefined),
targetGroupArns: new cdk.Token(() => this.targetGroupArns.length > 0 ? this.targetGroupArns : undefined),
tags: this.tags,
};

if (props.notificationsTopic) {
Expand Down Expand Up @@ -616,16 +606,6 @@ function renderRollingUpdateConfig(config: RollingUpdateConfiguration = {}): cdk
};
}

class TagManager extends cdk.TagManager {
protected tagFormatResolve(tagGroups: cdk.TagGroups): any {
const tags = {...tagGroups.nonStickyTags, ...tagGroups.ancestorTags, ...tagGroups.stickyTags};
return Object.keys(tags).map( (key) => {
const propagateAtLaunch = !!tagGroups.propagateTags[key] || !!tagGroups.ancestorTags[key];
return {key, value: tags[key], propagateAtLaunch};
});
}
}

/**
* Render a number of seconds to a PTnX string.
*/
Expand Down Expand Up @@ -748,4 +728,4 @@ export interface MetricTargetTrackingProps extends BaseTargetTrackingProps {
* Value to keep the metric around
*/
targetValue: number;
}
}
31 changes: 22 additions & 9 deletions packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import autoscaling = require('../lib');

export = {
'default fleet'(test: Test) {
const stack = new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' }});
const stack = getTestStack();
const vpc = mockVpc(stack);

new autoscaling.AutoScalingGroup(stack, 'MyFleet', {
Expand All @@ -18,6 +18,8 @@ export = {
vpc
});

stack.testInvokeAspects();

expect(stack).toMatch({
"Resources": {
"MyFleetInstanceSecurityGroup774E8234": {
Expand Down Expand Up @@ -365,7 +367,8 @@ export = {
},
'can set tags'(test: Test) {
// GIVEN
const stack = new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' }});
const stack = getTestStack();
// new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' }});
const vpc = mockVpc(stack);

// WHEN
Expand All @@ -378,27 +381,28 @@ export = {
minSuccessfulInstancesPercent: 50,
pauseTimeSec: 345
},
tags: {superfood: 'acai'},
});
asg.tags.setTag('notsuper', 'caramel', {propagate: false});
asg.apply( new cdk.Tag('superfood', 'acai'));
asg.apply( new cdk.Tag('notsuper', 'caramel', { applyToLaunchInstances: false }));

stack.testInvokeAspects();
// THEN
expect(stack).to(haveResource("AWS::AutoScaling::AutoScalingGroup", {
Tags: [
{
Key: 'superfood',
Value: 'acai',
Key: 'Name',
PropagateAtLaunch: true,
Value: 'MyFleet',
},
{
Key: 'Name',
Value: 'MyFleet',
Key: 'superfood',
PropagateAtLaunch: true,
Value: 'acai',
},
{
Key: 'notsuper',
Value: 'caramel',
PropagateAtLaunch: false,
Value: 'caramel',
},
]
}));
Expand All @@ -421,3 +425,12 @@ function mockSecurityGroup(stack: cdk.Stack) {
securityGroupId: 'most-secure',
});
}

class TestStack extends cdk.Stack {
public testInvokeAspects(): void {
this.invokeAspects();
}
}
function getTestStack(): TestStack {
return new TestStack(undefined, 'TestStack', { env: { account: '1234', region: 'us-east-1' } });
}
15 changes: 10 additions & 5 deletions packages/@aws-cdk/aws-autoscaling/test/test.scheduled-action.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource, MatchStyle } from '@aws-cdk/assert';
import { expect, haveResource, MatchStyle, TestStack } from '@aws-cdk/assert';
import ec2 = require('@aws-cdk/aws-ec2');
import cdk = require('@aws-cdk/cdk');
import { Test } from 'nodeunit';
Expand All @@ -7,7 +7,7 @@ import autoscaling = require('../lib');
export = {
'can schedule an action'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const stack = new TestStack();
const asg = makeAutoScalingGroup(stack);

// WHEN
Expand All @@ -27,7 +27,7 @@ export = {

'correctly formats date objects'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const stack = new TestStack();
const asg = makeAutoScalingGroup(stack);

// WHEN
Expand All @@ -47,7 +47,7 @@ export = {

'autoscaling group has recommended updatepolicy for scheduled actions'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const stack = getTestStack();
const asg = makeAutoScalingGroup(stack);

// WHEN
Expand All @@ -56,6 +56,7 @@ export = {
minCapacity: 10,
});

stack.testInvokeAspects();
// THEN
expect(stack).toMatch({
Resources: {
Expand Down Expand Up @@ -111,4 +112,8 @@ function makeAutoScalingGroup(scope: cdk.Construct) {
machineImage: new ec2.AmazonLinuxImage(),
updateType: autoscaling.UpdateType.RollingUpdate,
});
}
}

function getTestStack(): TestStack {
return new TestStack(undefined, 'TestStack', { env: { account: '1234', region: 'us-east-1' } });
}
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource, not } from '@aws-cdk/assert';
import { expect, haveResource, not, TestStack } from '@aws-cdk/assert';
import { Stack } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { CloudTrail, LogRetention, ReadWriteType } from '../lib';
Expand Down Expand Up @@ -147,5 +147,5 @@ export = {
};

function getTestStack(): Stack {
return new Stack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
return new TestStack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
}
9 changes: 1 addition & 8 deletions packages/@aws-cdk/aws-dynamodb/lib/table.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import appscaling = require('@aws-cdk/aws-applicationautoscaling');
import iam = require('@aws-cdk/aws-iam');
import cdk = require('@aws-cdk/cdk');
import { Construct, TagManager, Tags, Token } from '@aws-cdk/cdk';
import { Construct, Token } from '@aws-cdk/cdk';
import { CfnTable } from './dynamodb.generated';
import { EnableScalingProps, IScalableTableAttribute } from './scalable-attribute-api';
import { ScalableTableAttribute } from './scalable-table-attribute';
Expand Down Expand Up @@ -88,12 +88,6 @@ export interface TableProps {
*/
streamSpecification?: StreamViewType;

/**
* The AWS resource tags to associate with the table.
* @default undefined
*/
tags?: Tags;

/**
* The name of TTL attribute.
* @default undefined, TTL is disabled
Expand Down Expand Up @@ -216,7 +210,6 @@ export class Table extends Construct {
},
sseSpecification: props.sseEnabled ? { sseEnabled: props.sseEnabled } : undefined,
streamSpecification: props.streamSpecification ? { streamViewType: props.streamSpecification } : undefined,
tags: new TagManager(this, { initialTags: props.tags }),
timeToLiveSpecification: props.ttlAttributeName ? { attributeName: props.ttlAttributeName, enabled: true } : undefined
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { App, Stack } from '@aws-cdk/cdk';
import { App, Stack, Tag } from '@aws-cdk/cdk';
import { Attribute, AttributeType, BillingMode, ProjectionType, StreamViewType, Table } from '../lib';

// CDK parameters
Expand Down Expand Up @@ -48,11 +48,12 @@ const tableWithGlobalAndLocalSecondaryIndex = new Table(stack, TABLE_WITH_GLOBAL
pitrEnabled: true,
sseEnabled: true,
streamSpecification: StreamViewType.KeysOnly,
tags: { Environment: 'Production' },
billingMode: BillingMode.PayPerRequest,
ttlAttributeName: 'timeToLive'
});

tableWithGlobalAndLocalSecondaryIndex.apply(new Tag('Environment', 'Production'));

tableWithGlobalAndLocalSecondaryIndex.addPartitionKey(TABLE_PARTITION_KEY);
tableWithGlobalAndLocalSecondaryIndex.addSortKey(TABLE_SORT_KEY);
tableWithGlobalAndLocalSecondaryIndex.addGlobalSecondaryIndex({
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { App, Stack } from '@aws-cdk/cdk';
import { App, Stack, Tag } from '@aws-cdk/cdk';
import { Attribute, AttributeType, ProjectionType, StreamViewType, Table } from '../lib';

// CDK parameters
Expand Down Expand Up @@ -47,10 +47,10 @@ const tableWithGlobalAndLocalSecondaryIndex = new Table(stack, TABLE_WITH_GLOBAL
pitrEnabled: true,
sseEnabled: true,
streamSpecification: StreamViewType.KeysOnly,
tags: { Environment: 'Production' },
ttlAttributeName: 'timeToLive'
});

tableWithGlobalAndLocalSecondaryIndex.apply(new Tag('Environment', 'Production'));
tableWithGlobalAndLocalSecondaryIndex.addPartitionKey(TABLE_PARTITION_KEY);
tableWithGlobalAndLocalSecondaryIndex.addSortKey(TABLE_SORT_KEY);
tableWithGlobalAndLocalSecondaryIndex.addGlobalSecondaryIndex({
Expand Down
Loading