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

Framework: Tokens can be converted to strings #518

Merged
merged 20 commits into from
Aug 15, 2018
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, Stack, Token, tokenAwareJsonify } from "@aws-cdk/cdk";
import { CloudFormationJSON, Construct, Stack, Token } from "@aws-cdk/cdk";
import { cloudformation } from './cloudwatch.generated';
import { Column, Row } from "./layout";
import { IWidget } from "./widget";
Expand Down Expand Up @@ -33,7 +33,7 @@ export class Dashboard extends Construct {
dashboardBody: new Token(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This wraps a token and CloudFormationJSON.stringify also returns a Token. Can we eliminate one layer?

const column = new Column(...this.rows);
column.position(0, 0);
return tokenAwareJsonify({ widgets: column.toJson() });
return CloudFormationJSON.stringify({ widgets: column.toJson() });
})
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
"Type": "AWS::CloudWatch::Alarm",
"Properties": {
"ComparisonOperator": "GreaterThanOrEqualToThreshold",
"EvaluationPeriods": 3,
"MetricName": "ApproximateNumberOfMessagesVisible",
"Namespace": "AWS/SQS",
"Period": 300,
"Threshold": 100,
"Dimensions": [
{
"Name": "QueueName",
Expand All @@ -18,41 +23,55 @@
}
}
],
"EvaluationPeriods": 3,
"MetricName": "ApproximateNumberOfMessagesVisible",
"Namespace": "AWS/SQS",
"Period": 300,
"Statistic": "Average",
"Threshold": 100
"Statistic": "Average"
}
},
"DashCCD7F836": {
"Type": "AWS::CloudWatch::Dashboard",
"Properties": {
"DashboardName": "aws-cdk-cloudwatch-DashCCD7F836",
"DashboardBody": {
"Fn::Sub": [
"{\"widgets\":[{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":0,\"y\":0,\"properties\":{\"markdown\":\"# This is my dashboard\"}},{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":6,\"y\":0,\"properties\":{\"markdown\":\"you like?\"}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":2,\"properties\":{\"view\":\"timeSeries\",\"title\":\"Messages in queue\",\"region\":\"${ref0}\",\"annotations\":{\"alarms\":[\"${ref1}\"]},\"yAxis\":{\"left\":{\"min\":0}}}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":8,\"properties\":{\"view\":\"timeSeries\",\"title\":\"More messages in queue with alarm annotation\",\"region\":\"${ref0}\",\"metrics\":[[\"AWS/SQS\",\"ApproximateNumberOfMessagesVisible\",\"QueueName\",\"${ref2}\",{\"yAxis\":\"left\",\"period\":300,\"stat\":\"Average\"}]],\"annotations\":{\"horizontal\":[{\"label\":\"ApproximateNumberOfMessagesVisible >= 100 for 3 datapoints within 15 minutes\",\"value\":100,\"yAxis\":\"left\"}]},\"yAxis\":{\"left\":{\"min\":0},\"right\":{\"min\":0}}}},{\"type\":\"metric\",\"width\":6,\"height\":3,\"x\":0,\"y\":14,\"properties\":{\"view\":\"singleValue\",\"title\":\"Current messages in queue\",\"region\":\"${ref0}\",\"metrics\":[[\"AWS/SQS\",\"ApproximateNumberOfMessagesVisible\",\"QueueName\",\"${ref2}\",{\"yAxis\":\"left\",\"period\":300,\"stat\":\"Average\"}]]}}]}",
{
"ref0": {
"Fn::Join": [
"",
[
"{\"widgets\":[{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":0,\"y\":0,\"properties\":{\"markdown\":\"# This is my dashboard\"}},{\"type\":\"text\",\"width\":6,\"height\":2,\"x\":6,\"y\":0,\"properties\":{\"markdown\":\"you like?\"}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":2,\"properties\":{\"view\":\"timeSeries\",\"title\":\"Messages in queue\",\"region\":\"",
{
"Ref": "AWS::Region"
},
"ref1": {
"\",\"annotations\":{\"alarms\":[\"",
{
"Fn::GetAtt": [
"Alarm7103F465",
"Arn"
]
},
"ref2": {
"\"]},\"yAxis\":{\"left\":{\"min\":0}}}},{\"type\":\"metric\",\"width\":6,\"height\":6,\"x\":0,\"y\":8,\"properties\":{\"view\":\"timeSeries\",\"title\":\"More messages in queue with alarm annotation\",\"region\":\"",
{
"Ref": "AWS::Region"
},
"\",\"metrics\":[[\"AWS/SQS\",\"ApproximateNumberOfMessagesVisible\",\"QueueName\",\"",
{
"Fn::GetAtt": [
"queue",
"QueueName"
]
}
}
},
"\",{\"yAxis\":\"left\",\"period\":300,\"stat\":\"Average\"}]],\"annotations\":{\"horizontal\":[{\"label\":\"ApproximateNumberOfMessagesVisible >= 100 for 3 datapoints within 15 minutes\",\"value\":100,\"yAxis\":\"left\"}]},\"yAxis\":{\"left\":{\"min\":0},\"right\":{\"min\":0}}}},{\"type\":\"metric\",\"width\":6,\"height\":3,\"x\":0,\"y\":14,\"properties\":{\"view\":\"singleValue\",\"title\":\"Current messages in queue\",\"region\":\"",
{
"Ref": "AWS::Region"
},
"\",\"metrics\":[[\"AWS/SQS\",\"ApproximateNumberOfMessagesVisible\",\"QueueName\",\"",
{
"Fn::GetAtt": [
"queue",
"QueueName"
]
},
"\",{\"yAxis\":\"left\",\"period\":300,\"stat\":\"Average\"}]]}}]}"
]
]
}
},
"DashboardName": "aws-cdk-cloudwatch-DashCCD7F836"
}
}
}
}
}
15 changes: 6 additions & 9 deletions packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export = {
test.done();
},

'tokens in widgets are retained through FnSub'(test: Test) {
'tokens in widgets are retained'(test: Test) {
// GIVEN
const stack = new Stack();
const dashboard = new Dashboard(stack, 'Dash');
Expand All @@ -82,14 +82,11 @@ export = {

// THEN
expect(stack).to(haveResource('AWS::CloudWatch::Dashboard', {
DashboardBody: { "Fn::Sub": [
// tslint:disable-next-line:max-line-length
"{\"widgets\":[{\"type\":\"metric\",\"width\":1,\"height\":1,\"x\":0,\"y\":0,\"properties\":{\"view\":\"timeSeries\",\"region\":\"${ref0}\",\"metrics\":[],\"annotations\":{\"horizontal\":[]},\"yAxis\":{\"left\":{\"min\":0},\"right\":{\"min\":0}}}}]}",
{
ref0: { Ref: "AWS::Region" }
}
]
}
DashboardBody: { "Fn::Join": [ "", [
"{\"widgets\":[{\"type\":\"metric\",\"width\":1,\"height\":1,\"x\":0,\"y\":0,\"properties\":{\"view\":\"timeSeries\",\"region\":\"",
{ Ref: "AWS::Region" },
"\",\"metrics\":[],\"annotations\":{\"horizontal\":[]},\"yAxis\":{\"left\":{\"min\":0},\"right\":{\"min\":0}}}}]}"
]]}
}));

test.done();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export = {
new codepipeline.GitHubSource(new codepipeline.Stage(p, 'Source'), 'GH', {
artifactName: 'A',
branch: 'branch',
oauthToken: secret,
oauthToken: secret.value,
owner: 'foo',
repo: 'bar'
});
Expand Down
4 changes: 1 addition & 3 deletions packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,12 @@ export class CrossAccountDestination extends cdk.Construct implements ILogSubscr
constructor(parent: cdk.Construct, id: string, props: CrossAccountDestinationProps) {
super(parent, id);

this.policyDocument = new cdk.PolicyDocument();

// In the underlying model, the name is not optional, but we make it so anyway.
const destinationName = props.destinationName || new cdk.Token(() => this.generateUniqueName());

this.resource = new cloudformation.DestinationResource(this, 'Resource', {
destinationName,
destinationPolicy: new cdk.Token(() => !this.policyDocument.isEmpty ? JSON.stringify(this.policyDocument.resolve()) : ""),
destinationPolicy: this.policyDocument,
roleArn: props.role.roleArn,
targetArn: props.targetArn
});
Expand Down
3 changes: 1 addition & 2 deletions packages/@aws-cdk/aws-logs/test/test.destination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ export = {

// THEN
expect(stack).to(haveResource('AWS::Logs::Destination', (props: any) => {
// tslint:disable-next-line:no-console
const pol = JSON.parse(props.DestinationPolicy);

return pol.Statement[0].action[0] === 'logs:TalkToMe';
return pol.Statement[0].Action === 'logs:TalkToMe';
}));

test.done();
Expand Down
83 changes: 83 additions & 0 deletions packages/@aws-cdk/cdk/lib/cloudformation/cloudformation-json.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { resolve, Token } from "../core/tokens";
import { CloudFormationToken, isIntrinsic } from "./cloudformation-token";

/**
* Class for JSON routines that are framework-aware
*/
export class CloudFormationJSON {
/**
* Turn an arbitrary structure potentially containing Tokens into JSON.
Copy link
Contributor

Choose a reason for hiding this comment

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

Return type should be string correct?
Also, in the description say "into a JSON string"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually returns a Token<string>.

*/
public static stringify(obj: any): any {
return new Token(() => {
// Resolve inner value so that if they evaluate to literals, we maintain the type.
//
// Then replace intrinsics with a special sublcass of Token that overrides toJSON() to
Copy link
Contributor

Choose a reason for hiding this comment

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

"sublcass" => "subclass", "to and" => "and" (i think)

// and deep-escapes the intrinsic, so if we resolve() the strings again it evaluates
// to the right string.
const resolved = resolve(obj);

// We can just directly return this value, since resolve() will be called
// on our return value anyway.
return JSON.stringify(deepReplaceIntrinsics(resolved));
});

/**
* Recurse into a structure, replace all intrinsics with
Copy link
Contributor

Choose a reason for hiding this comment

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

"with..."?

*/
function deepReplaceIntrinsics(x: any): any {
if (isIntrinsic(x)) {
return wrapIntrinsic(x);
}

if (Array.isArray(x)) {
return x.map(deepReplaceIntrinsics);
}

if (typeof x === 'object') {
for (const key of Object.keys(x)) {
x[key] = deepReplaceIntrinsics(x[key]);
}
}

return x;
}

function wrapIntrinsic(intrinsic: any): IntrinsicToken {
return new IntrinsicToken(() => deepQuoteStringsForJSON(intrinsic));
}
}
}

class IntrinsicToken extends CloudFormationToken {
/**
* Special handler that gets called when JSON.stringify() is used.
*/
public toJSON() {
return this.toString();
}
}

/**
* Deep escape strings for use in a JSON context
*/
function deepQuoteStringsForJSON(x: any): any {
if (typeof x === 'string') {
// Whenever we escape a string we strip off the outermost quotes
// since we're already in a quoted context.
const stringified = JSON.stringify(x);
return stringified.substring(1, stringified.length - 1);
}

if (Array.isArray(x)) {
return x.map(deepQuoteStringsForJSON);
}

if (typeof x === 'object') {
for (const key of Object.keys(x)) {
x[key] = deepQuoteStringsForJSON(x[key]);
}
}

return x;
}
38 changes: 38 additions & 0 deletions packages/@aws-cdk/cdk/lib/cloudformation/cloudformation-token.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { Token } from "../core/tokens";

/**
* Base class for CloudFormation built-ins
*/
export class CloudFormationToken extends Token {
constructor(valueOrFunction: any, stringRepresentationHint?: string) {
super(valueOrFunction, {
joiner: CLOUDFORMATION_JOINER,
stringRepresentationHint
});
}
}

import { FnConcat } from "./fn";

/**
* The default intrinsics Token engine for CloudFormation
*/
export const CLOUDFORMATION_JOINER = {
joinerName: 'CloudFormation',

joinStringFragments(fragments: any[]): any {
return new FnConcat(...fragments.map(x => isIntrinsic(x) ? x : `${x}`));
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of ${x} maybe x.toString() will be more readable, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That fails in the case of undefined and null, whereas the quoting will work correctly for those cases.

}
};

/**
* Return whether the given value represents a CloudFormation intrinsic
*/
export function isIntrinsic(x: any) {
if (Array.isArray(x) || x === null || typeof x !== 'object') { return false; }
Copy link
Contributor

Choose a reason for hiding this comment

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

use == null to capture undefined too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But typeof undefined !== 'object', so that's already caught.


const keys = Object.keys(x);
if (keys.length !== 1) { return false; }

return keys[0] === 'Ref' || keys[0].startsWith('Fn::');
}
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cdk/lib/cloudformation/fn.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Token } from '../core/tokens';
import { CloudFormationToken } from './cloudformation-token';
// tslint:disable:max-line-length

/**
* CloudFormation intrinsic functions.
* http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html
*/
export class Fn extends Token {
export class Fn extends CloudFormationToken {
constructor(name: string, value: any) {
super(() => ({ [name]: value }));
}
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/cdk/lib/cloudformation/pseudo.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Token } from '../core/tokens';
import { CloudFormationToken } from './cloudformation-token';

export class PseudoParameter extends Token {
export class PseudoParameter extends CloudFormationToken {
constructor(name: string) {
super(() => ({ Ref: name }));
super(() => ({ Ref: name }), name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need to be lazy? We can plug in a concrete value

}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/cdk/lib/cloudformation/resource.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Construct } from '../core/construct';
import { Token } from '../core/tokens';
import { capitalizePropertyNames, ignoreEmpty } from '../core/util';
import { CloudFormationToken } from './cloudformation-token';
import { Condition } from './condition';
import { CreationPolicy, DeletionPolicy, UpdatePolicy } from './resource-policy';
import { IDependable, Referenceable, StackElement } from './stack';
Expand Down Expand Up @@ -82,7 +83,7 @@ export class Resource extends Referenceable {
* @param attributeName The name of the attribute.
*/
public getAtt(attributeName: string): Token {
return new Token(() => ({ 'Fn::GetAtt': [this.logicalId, attributeName] }));
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn’t need to be lazy

return new CloudFormationToken(() => ({ 'Fn::GetAtt': [this.logicalId, attributeName] }), `${this.logicalId}.${attributeName}`);
}

/**
Expand Down
3 changes: 1 addition & 2 deletions packages/@aws-cdk/cdk/lib/cloudformation/rule.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Construct } from '../core/construct';
import { Token } from '../core/tokens';
import { capitalizePropertyNames } from '../core/util';
import { FnCondition } from './fn';
import { Referenceable } from './stack';
Expand Down Expand Up @@ -30,7 +29,7 @@ export interface RuleProps {
* If the rule condition evaluates to false, the rule doesn't take effect.
* If the function in the rule condition evaluates to true, expressions in each assert are evaluated and applied.
*/
ruleCondition?: Token;
ruleCondition?: FnCondition;

/**
* Assertions which define the rule.
Expand Down
9 changes: 2 additions & 7 deletions packages/@aws-cdk/cdk/lib/cloudformation/secret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ export interface SecretProps {
*/
export class SecretParameter extends Construct {
/**
* A token for the secret value.
* The value of the secret parameter.
*/
public value: Token;
public value: Secret;

constructor(parent: Construct, name: string, props: SecretProps) {
super(parent, name);
Expand All @@ -93,9 +93,4 @@ export class SecretParameter extends Construct {

this.value = param.ref;
}

// implicitly implements Token, and therefore Secret.
public resolve(): any {
return this.value;
}
}
5 changes: 3 additions & 2 deletions packages/@aws-cdk/cdk/lib/cloudformation/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { App } from '../app';
import { Construct, PATH_SEP } from '../core/construct';
import { resolve, Token } from '../core/tokens';
import { Environment } from '../environment';
import { CloudFormationToken } from './cloudformation-token';
import { HashedAddressingScheme, IAddressingScheme, LogicalIDs } from './logical-id';
import { Resource } from './resource';

Expand Down Expand Up @@ -391,8 +392,8 @@ export abstract class Referenceable extends StackElement {
/**
* Returns a token to a CloudFormation { Ref } that references this entity based on it's logical ID.
*/
public get ref() {
return new Token(() => ({ Ref: this.logicalId }));
public get ref(): Token {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should just define a ‘Ref’ class for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need for that. It doesn't add a lot of value and no user is ever going to instantiate it.

return new CloudFormationToken(() => ({ Ref: this.logicalId }), `${this.logicalId}.Ref`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn’t need to be lazy

}
}

Expand Down
Loading