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: no more generated attribute types in CFN layer (L1) #1489

Merged
merged 6 commits into from
Jan 8, 2019
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Stop generating "| Token" in L1 properties that accept "string" or "s…
…tring[]"
  • Loading branch information
Elad Ben-Israel committed Jan 7, 2019
commit 62f35be432d760a42ab0f20015239ec72d74d985
Original file line number Diff line number Diff line change
@@ -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', {
Original file line number Diff line number Diff line change
@@ -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: [
{
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
Original file line number Diff line number Diff line change
@@ -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: [
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/lib/policy.ts
Original file line number Diff line number Diff line change
@@ -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)),
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts
Original file line number Diff line number Diff line change
@@ -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();
}
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-route53/lib/hosted-zone.ts
Original file line number Diff line number Diff line change
@@ -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) {
Original file line number Diff line number Diff line change
@@ -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.
29 changes: 27 additions & 2 deletions tools/cfn2ts/lib/codegen.ts
Original file line number Diff line number Diff line change
@@ -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;
}