Skip to content

Commit

Permalink
fix(cfn2ts): properly de-Tokenize L1 string-arrays (#2033)
Browse files Browse the repository at this point in the history
Because of accepted Tokenized values, cfn2ts previously translated a
string and string array into:

    interface Props {
        someString: string;
        stringArray: Array<string | Token> | Token;
    }

The latter was not intended. We now emit:

    interface Props {
        someString: string;
        stringArray: string[];
    }

Since the Token will be typed as `Object` in Java, this change now makes
it impossible to mistakenly pass any old java Object. A typical case
would be pass a `Subnet` where a `string subnetId` was expected, failing
in a nonobvious way.

Fixes #2030.
  • Loading branch information
rix0rrr authored Mar 18, 2019
1 parent 23509ae commit 1e50383
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 24 deletions.
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
imageId: machineImage.imageId,
keyName: props.keyName,
instanceType: props.instanceType.toString(),
securityGroups: securityGroupsToken,
securityGroups: securityGroupsToken.toList(),
iamInstanceProfile: iamProfile.ref,
userData: userDataToken,
associatePublicIpAddress: props.associatePublicIpAddress,
Expand All @@ -274,8 +274,8 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
maxSize: maxCapacity.toString(),
desiredCapacity: desiredCapacity.toString(),
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),
loadBalancerNames: new cdk.Token(() => this.loadBalancerNames.length > 0 ? this.loadBalancerNames : undefined).toList(),
targetGroupArns: new cdk.Token(() => this.targetGroupArns.length > 0 ? this.targetGroupArns : undefined).toList(),
};

if (props.notificationsTopic) {
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ export class Alarm extends Construct {

// Actions
actionsEnabled: props.actionsEnabled,
alarmActions: new Token(() => this.alarmActionArns),
insufficientDataActions: new Token(() => this.insufficientDataActionArns),
okActions: new Token(() => this.okActionArns),
alarmActions: new Token(() => this.alarmActionArns).toList(),
insufficientDataActions: new Token(() => this.insufficientDataActionArns).toList(),
okActions: new Token(() => this.okActionArns).toList(),

// Metric
...metricJson(props.metric)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupBase {
autoScalingGroups: new cdk.Token(() =>
this._autoScalingGroups.length === 0
? undefined
: this._autoScalingGroups.map(asg => asg.autoScalingGroupName)),
: this._autoScalingGroups.map(asg => asg.autoScalingGroupName)).toList(),
loadBalancerInfo: this.loadBalancerInfo(props.loadBalancer),
deploymentStyle: props.loadBalancer === undefined
? undefined
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export abstract class BaseService extends cdk.Construct
awsvpcConfiguration: {
assignPublicIp: assignPublicIp ? 'ENABLED' : 'DISABLED',
subnets: subnets.map(x => x.subnetId),
securityGroups: new cdk.Token(() => [securityGroup!.securityGroupId]),
securityGroups: new cdk.Token(() => [securityGroup!.securityGroupId]).toList(),
}
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/lib/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class Group extends Construct implements IPrincipal {
*/
public readonly principal: PolicyPrincipal;

private readonly managedPolicies: any[];
private readonly managedPolicies: string[];
private readonly attachedPolicies = new AttachedPolicies();
private defaultPolicy?: Policy;

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { Policy } from './policy';

const MAX_POLICY_NAME_LEN = 128;

export function undefinedIfEmpty<T>(f: () => T[]): Token {
export function undefinedIfEmpty(f: () => string[]): string[] {
return new Token(() => {
const array = f();
return (array && array.length > 0) ? array : undefined;
});
}).toList();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ export class Function extends FunctionBase {
functionName: props.functionName,
description: props.description,
code: new cdk.Token(() => props.code._toJSON(resource)),
layers: new cdk.Token(() => this.layers.length > 0 ? this.layers.map(layer => layer.layerVersionArn) : undefined),
layers: new cdk.Token(() => this.layers.length > 0 ? this.layers.map(layer => layer.layerVersionArn) : undefined).toList(),
handler: props.handler,
timeout: props.timeout,
runtime: props.runtime.name,
Expand Down
30 changes: 18 additions & 12 deletions tools/cfn2ts/lib/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,8 +579,11 @@ export default class CodeGenerator {
// render the union of all item types
const itemTypes = genspec.specTypesToCodeTypes(resourceContext, itemTypeNames(propSpec));

if (propName !== 'Tags') {
// Always accept a token in place of any list element
// 'tokenizableType' operates at the level of rendered type names in TypeScript, so stringify
// the objects.
const renderedTypes = itemTypes.map(t => this.renderCodeName(resourceContext, t));
if (!tokenizableType(renderedTypes) && propName !== 'Tags') {
// Always accept a token in place of any list element (unless the list elements are tokenizable)
itemTypes.push(genspec.TOKEN_NAME);
}

Expand Down Expand Up @@ -619,17 +622,20 @@ export default class CodeGenerator {
return alternatives.join(' | ');
}

/**
* Render a CodeName to a string representation of it in TypeScript
*/
private renderCodeName(context: genspec.CodeName, type: genspec.CodeName): string {
const rel = type.relativeTo(context);
const specType = rel.specName && this.spec.PropertyTypes[rel.specName.fqn];
if (!specType || schema.isPropertyBag(specType)) {
return rel.fqn;
}
return this.findNativeType(context, specType);
}

private renderTypeUnion(context: genspec.CodeName, types: genspec.CodeName[]) {
return types
.map(type => type.relativeTo(context))
.map(type => {
const specType = type.specName && this.spec.PropertyTypes[type.specName.fqn];
if (!specType || schema.isPropertyBag(specType)) {
return type.fqn;
}
return this.findNativeType(context, specType);
})
.join(' | ');
return types.map(t => this.renderCodeName(context, t)).join(' | ');
}

private docLink(link: string | undefined, ...before: string[]) {
Expand Down

0 comments on commit 1e50383

Please sign in to comment.