From 62f35be432d760a42ab0f20015239ec72d74d985 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 7 Jan 2019 16:59:17 +0200 Subject: [PATCH] Stop generating "| Token" in L1 properties that accept "string" or "string[]" --- .../aws-autoscaling/lib/auto-scaling-group.ts | 2 +- .../lib/github-source-action.ts | 2 +- .../aws-ecs/lib/base/task-definition.ts | 2 +- packages/@aws-cdk/aws-iam/lib/policy.ts | 2 +- .../aws-logs/lib/cross-account-destination.ts | 8 ++--- .../@aws-cdk/aws-route53/lib/hosted-zone.ts | 2 +- .../lib/cloudformation/cloudformation-json.ts | 4 +-- tools/cfn2ts/lib/codegen.ts | 29 +++++++++++++++++-- 8 files changed, 38 insertions(+), 13 deletions(-) diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts index 19f50f187bfed..dbd85436c8000 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -222,7 +222,7 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup // use delayed evaluation const machineImage = props.machineImage.getImage(this); - const userDataToken = new cdk.Token(() => cdk.Fn.base64((machineImage.os.createUserData(this.userDataLines)))); + const userDataToken = new cdk.Token(() => cdk.Fn.base64((machineImage.os.createUserData(this.userDataLines)))).toString(); const securityGroupsToken = new cdk.Token(() => this.securityGroups.map(sg => sg.securityGroupId)); const launchConfig = new CfnLaunchConfiguration(this, 'LaunchConfig', { diff --git a/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts b/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts index 652b13e6bbcfa..c70c2d048ddf7 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/github-source-action.ts @@ -76,7 +76,7 @@ export class GitHubSourceAction extends actions.SourceAction { new CfnWebhook(this, 'WebhookResource', { authentication: 'GITHUB_HMAC', authenticationConfiguration: { - secretToken: props.oauthToken, + secretToken: props.oauthToken.toString(), }, filters: [ { diff --git a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts index f1a96595e6640..4c197604b76d2 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts @@ -188,7 +188,7 @@ export class TaskDefinition extends cdk.Construct { const taskDef = new CfnTaskDefinition(this, 'Resource', { containerDefinitions: new cdk.Token(() => this.containers.map(x => x.renderContainerDefinition())), volumes: new cdk.Token(() => this.volumes), - executionRoleArn: new cdk.Token(() => this.executionRole && this.executionRole.roleArn), + executionRoleArn: new cdk.Token(() => this.executionRole && this.executionRole.roleArn).toString(), family: this.family, taskRoleArn: this.taskRole.roleArn, requiresCompatibilities: [ diff --git a/packages/@aws-cdk/aws-iam/lib/policy.ts b/packages/@aws-cdk/aws-iam/lib/policy.ts index 334d80205116c..d3a5104a02b9e 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy.ts @@ -108,7 +108,7 @@ export class Policy extends Construct implements IDependable { const resource = new CfnPolicy(this, 'Resource', { policyDocument: this.document, - policyName: new Token(() => this.policyName), + policyName: new Token(() => this.policyName).toString(), roles: undefinedIfEmpty(() => this.roles.map(r => r.roleName)), users: undefinedIfEmpty(() => this.users.map(u => u.userName)), groups: undefinedIfEmpty(() => this.groups.map(g => g.groupName)), diff --git a/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts b/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts index 881f6e0769da0..4507f01808926 100644 --- a/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts +++ b/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts @@ -60,12 +60,12 @@ export class CrossAccountDestination extends cdk.Construct implements ILogSubscr super(scope, id); // In the underlying model, the name is not optional, but we make it so anyway. - const destinationName = props.destinationName || new cdk.Token(() => this.generateUniqueName()); + const destinationName = props.destinationName || new cdk.Token(() => this.generateUniqueName()).toString(); this.resource = new CfnDestination(this, 'Resource', { destinationName, // Must be stringified policy - destinationPolicy: new cdk.Token(() => this.stringifiedPolicyDocument()), + destinationPolicy: this.lazyStringifiedPolicyDocument(), roleArn: props.role.roleArn, targetArn: props.targetArn }); @@ -94,7 +94,7 @@ export class CrossAccountDestination extends cdk.Construct implements ILogSubscr /** * Return a stringified JSON version of the PolicyDocument */ - private stringifiedPolicyDocument() { - return this.policyDocument.isEmpty ? '' : cdk.CloudFormationJSON.stringify(cdk.resolve(this.policyDocument)); + private lazyStringifiedPolicyDocument() { + return new cdk.Token(() => this.policyDocument.isEmpty ? '' : cdk.CloudFormationJSON.stringify(cdk.resolve(this.policyDocument))).toString(); } } diff --git a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts index 37bd097d253e7..2301d78b33291 100644 --- a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts +++ b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts @@ -137,7 +137,7 @@ export class PrivateHostedZone extends HostedZone { } function toVpcProperty(vpc: ec2.IVpcNetwork): CfnHostedZone.VPCProperty { - return { vpcId: vpc.vpcId, vpcRegion: new cdk.AwsRegion() }; + return { vpcId: vpc.vpcId, vpcRegion: new cdk.AwsRegion().toString() }; } function determineHostedZoneProps(props: PublicHostedZoneProps) { diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/cloudformation-json.ts b/packages/@aws-cdk/cdk/lib/cloudformation/cloudformation-json.ts index 6075f5650352a..670a974a1b224 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation/cloudformation-json.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation/cloudformation-json.ts @@ -15,7 +15,7 @@ export class CloudFormationJSON { * All Tokens substituted in this way must return strings, or the evaluation * in CloudFormation will fail. */ - public static stringify(obj: any): Token { + public static stringify(obj: any): string { return new Token(() => { // Resolve inner value first so that if they evaluate to literals, we // maintain the type (and discard 'undefined's). @@ -31,7 +31,7 @@ export class CloudFormationJSON { // We can just directly return this value, since resolve() will be called // on our return value anyway. return JSON.stringify(deepReplaceIntrinsics(resolved)); - }); + }).toString(); /** * Recurse into a structure, replace all intrinsics with IntrinsicTokens. diff --git a/tools/cfn2ts/lib/codegen.ts b/tools/cfn2ts/lib/codegen.ts index 37da0266521c5..03e0a33f52ff6 100644 --- a/tools/cfn2ts/lib/codegen.ts +++ b/tools/cfn2ts/lib/codegen.ts @@ -565,8 +565,14 @@ export default class CodeGenerator { alternatives.push(this.renderTypeUnion(resourceContext, types)); } - // Always - alternatives.push(genspec.TOKEN_NAME.fqn); + // Only if this property is not of a "tokenizable type" (string, string[], + // number in the future) we add a type union for `cdk.Token`. We rather + // everything to be tokenizable because there are languages that do not + // support union types (i.e. Java, .NET), so we lose type safety if we have + // a union. + if (!tokenizableType(alternatives)) { + alternatives.push(genspec.TOKEN_NAME.fqn); + } return alternatives.join(' | '); } @@ -618,3 +624,22 @@ function mapperNames(types: genspec.CodeName[]): string { function quoteCode(code: string): string { return '``' + code + '``'; } + +function tokenizableType(alternatives: string[]) { + if (alternatives.length > 1) { + return false; + } + + const type = alternatives[0]; + if (type === 'string') { + return true; + } + + if (type === 'string[]') { + return true; + } + + // TODO: number + + return false; +} \ No newline at end of file