Skip to content

Commit 277cfe2

Browse files
leonmk-awsLeon Michalski
authored andcommitted
chore(spec2cdk): ensure strings are passed to attributes (#35911)
### Issue # (if applicable) Closes #<issue number here>. ### Reason for this change In some languages for which jsii cannot properly generate unions like Go, if a user passes the wrong property (i.e an `ILayerRef` instead of an `IRoleRef`), the error message will be unclear: ``` panic: ValidationError: Resolution error: Resolution error: Trying to resolve() a Construct at /Resources/${Token[GoAppStack.SampleFunction.LogicalID.139]}/Properties/role/node.. ``` ### Description of changes Add a check to ensure that all unions have been resolved to a string and throw an error if not, at runtime it will look like: ``` panic: TypeError: Property role should be one of iam.IRoleRef | string ``` ### Description of how you validated changes Ran locally, updated snapshot tests. Unit test will be added once the relationships are enabled for `lambda`. ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent f31b6da commit 277cfe2

File tree

4 files changed

+37
-18
lines changed

4 files changed

+37
-18
lines changed

packages/aws-cdk-lib/core/lib/runtime.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,3 +400,14 @@ function isCloudFormationDynamicReference(x: any) {
400400
class CfnSynthesisError extends Error {
401401
public readonly type = 'CfnSynthesisError';
402402
}
403+
404+
/**
405+
* Ensures that a property is either undefined or a string.
406+
* Used in spec2cdk to have better error messages in other languages.
407+
*/
408+
export function ensureStringOrUndefined(value: any, propName: string, possibleType: string): string | undefined {
409+
if (value !== undefined && typeof value !== 'string') {
410+
throw new TypeError(`Property ${propName} should be one of ${possibleType}`);
411+
}
412+
return value;
413+
}

tools/@aws-cdk/spec2cdk/lib/cdk/relationship-decider.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export interface Relationship {
1818
readonly referenceName: string;
1919
/** The property to extract from the reference object (e.g. "roleArn") */
2020
readonly propName: string;
21+
/** Human friendly name of the reference type for error generation (e.g. "iam.IRoleRef") */
22+
readonly typeDisplayName: string;
2123
}
2224

2325
/**
@@ -115,6 +117,7 @@ export class RelationshipDecider {
115117
referenceType: aliasedTypeName ?? interfaceName,
116118
referenceName: refPropStructName,
117119
propName: referencePropertyName(relationship.propertyName, targetResource.name),
120+
typeDisplayName: `${typeAliasPrefixFromResource(targetResource).toLowerCase()}.${interfaceName}`,
118121
});
119122
}
120123
return parsedRelationships;

tools/@aws-cdk/spec2cdk/lib/cdk/resolver-builder.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ export class ResolverBuilder {
7474
? Type.arrayOf(Type.distinctUnionOf(resolvableType.arrayOfType, ...newTypes))
7575
: Type.distinctUnionOf(resolvableType, ...newTypes);
7676

77+
const typeDisplayNames = [
78+
...relationships.map(r => r.typeDisplayName),
79+
resolvableType.arrayOfType?.toString() ?? resolvableType.toString(),
80+
].join(' | ');
81+
7782
// Generates code like:
7883
// For single value: (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? (props.roleArn as IUserRef)?.userRef?.userArn ?? props.roleArn
7984
// For array: props.roleArns?.map((item: any) => (item as IRoleRef)?.roleRef?.roleArn ?? (item as IUserRef)?.userRef?.userArn ?? item)
@@ -84,7 +89,7 @@ export class ResolverBuilder {
8489
const buildChain = (itemName: string) => [
8590
...[...arnRels, ...otherRels]
8691
.map(r => `(${itemName} as ${r.referenceType})?.${r.referenceName}?.${r.propName}`),
87-
itemName,
92+
`cdk.ensureStringOrUndefined(${itemName}, "${name}", "${typeDisplayNames}")`,
8893
].join(' ?? ');
8994
const resolver = (_: Expression) => {
9095
if (resolvableType.arrayOfType) {

tools/@aws-cdk/spec2cdk/test/__snapshots__/relationships.test.ts.snap

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import * as cdk_errors from "aws-cdk-lib/core/lib/errors";
1212
*
1313
* @stability experimental
1414
*/
15-
export interface IRoleRef extends constructs.IConstruct {
15+
export interface IRoleRef extends constructs.IConstruct, cdk.IEnvironmentAware {
1616
/**
1717
* A reference to a Role resource.
1818
*/
@@ -165,7 +165,7 @@ function CfnRolePropsFromCloudFormation(properties: any): cfn_parse.FromCloudFor
165165
*
166166
* @stability experimental
167167
*/
168-
export interface IResourceRef extends constructs.IConstruct {
168+
export interface IResourceRef extends constructs.IConstruct, cdk.IEnvironmentAware {
169169
/**
170170
* A reference to a Resource resource.
171171
*/
@@ -280,7 +280,7 @@ export interface CfnResourceProps {
280280
function flattenCfnResourcePermissionProperty(props: cdk.IResolvable | CfnResource.PermissionProperty): cdk.IResolvable | CfnResource.PermissionProperty {
281281
if (cdk.isResolvableObject(props)) return props;
282282
return {
283-
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? props.roleArn
283+
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? cdk.ensureStringOrUndefined(props.roleArn, "roleArn", "iam.IRoleRef | string")
284284
};
285285
}
286286
@@ -391,7 +391,7 @@ import * as cdk_errors from "aws-cdk-lib/core/lib/errors";
391391
*
392392
* @stability experimental
393393
*/
394-
export interface IRoleRef extends constructs.IConstruct {
394+
export interface IRoleRef extends constructs.IConstruct, cdk.IEnvironmentAware {
395395
/**
396396
* A reference to a Role resource.
397397
*/
@@ -544,7 +544,7 @@ function CfnRolePropsFromCloudFormation(properties: any): cfn_parse.FromCloudFor
544544
*
545545
* @stability experimental
546546
*/
547-
export interface IUserRef extends constructs.IConstruct {
547+
export interface IUserRef extends constructs.IConstruct, cdk.IEnvironmentAware {
548548
/**
549549
* A reference to a User resource.
550550
*/
@@ -697,7 +697,7 @@ function CfnUserPropsFromCloudFormation(properties: any): cfn_parse.FromCloudFor
697697
*
698698
* @stability experimental
699699
*/
700-
export interface IPolicyRef extends constructs.IConstruct {
700+
export interface IPolicyRef extends constructs.IConstruct, cdk.IEnvironmentAware {
701701
/**
702702
* A reference to a Policy resource.
703703
*/
@@ -752,7 +752,7 @@ export class CfnPolicy extends cdk.CfnResource implements cdk.IInspectable, IPol
752752
"properties": props
753753
});
754754
755-
this.principalArn = (props.principalArn as IRoleRef)?.roleRef?.roleArn ?? (props.principalArn as IUserRef)?.userRef?.userArn ?? props.principalArn;
755+
this.principalArn = (props.principalArn as IRoleRef)?.roleRef?.roleArn ?? (props.principalArn as IUserRef)?.userRef?.userArn ?? cdk.ensureStringOrUndefined(props.principalArn, "principalArn", "iam.IRoleRef | iam.IUserRef | string");
756756
}
757757
758758
public get policyRef(): PolicyReference {
@@ -859,7 +859,7 @@ import * as cdk_errors from "aws-cdk-lib/core/lib/errors";
859859
*
860860
* @stability experimental
861861
*/
862-
export interface IRoleRef extends constructs.IConstruct {
862+
export interface IRoleRef extends constructs.IConstruct, cdk.IEnvironmentAware {
863863
/**
864864
* A reference to a Role resource.
865865
*/
@@ -1012,7 +1012,7 @@ function CfnRolePropsFromCloudFormation(properties: any): cfn_parse.FromCloudFor
10121012
*
10131013
* @stability experimental
10141014
*/
1015-
export interface ITaskRef extends constructs.IConstruct {
1015+
export interface ITaskRef extends constructs.IConstruct, cdk.IEnvironmentAware {
10161016
/**
10171017
* A reference to a Task resource.
10181018
*/
@@ -1127,7 +1127,7 @@ export interface CfnTaskProps {
11271127
function flattenCfnTaskExecutionConfigProperty(props: CfnTask.ExecutionConfigProperty | cdk.IResolvable): CfnTask.ExecutionConfigProperty | cdk.IResolvable {
11281128
if (cdk.isResolvableObject(props)) return props;
11291129
return {
1130-
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? props.roleArn
1130+
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? cdk.ensureStringOrUndefined(props.roleArn, "roleArn", "iam.IRoleRef | string")
11311131
};
11321132
}
11331133
@@ -1238,7 +1238,7 @@ import * as cdk_errors from "aws-cdk-lib/core/lib/errors";
12381238
*
12391239
* @stability experimental
12401240
*/
1241-
export interface IRoleRef extends constructs.IConstruct {
1241+
export interface IRoleRef extends constructs.IConstruct, cdk.IEnvironmentAware {
12421242
/**
12431243
* A reference to a Role resource.
12441244
*/
@@ -1391,7 +1391,7 @@ function CfnRolePropsFromCloudFormation(properties: any): cfn_parse.FromCloudFor
13911391
*
13921392
* @stability experimental
13931393
*/
1394-
export interface IJobRef extends constructs.IConstruct {
1394+
export interface IJobRef extends constructs.IConstruct, cdk.IEnvironmentAware {
13951395
/**
13961396
* A reference to a Job resource.
13971397
*/
@@ -1523,7 +1523,7 @@ export interface CfnJobProps {
15231523
function flattenCfnJobConfigProperty(props: CfnJob.ConfigProperty | cdk.IResolvable): CfnJob.ConfigProperty | cdk.IResolvable {
15241524
if (cdk.isResolvableObject(props)) return props;
15251525
return {
1526-
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? props.roleArn,
1526+
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? cdk.ensureStringOrUndefined(props.roleArn, "roleArn", "iam.IRoleRef | string"),
15271527
"timeout": props.timeout
15281528
};
15291529
}
@@ -1577,7 +1577,7 @@ function CfnJobConfigPropertyFromCloudFormation(properties: any): cfn_parse.From
15771577
function flattenCfnJobOldConfigProperty(props: cdk.IResolvable | CfnJob.OldConfigProperty): cdk.IResolvable | CfnJob.OldConfigProperty {
15781578
if (cdk.isResolvableObject(props)) return props;
15791579
return {
1580-
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? props.roleArn
1580+
"roleArn": (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? cdk.ensureStringOrUndefined(props.roleArn, "roleArn", "iam.IRoleRef | string")
15811581
};
15821582
}
15831583
@@ -1688,7 +1688,7 @@ import * as cdk_errors from "aws-cdk-lib/core/lib/errors";
16881688
*
16891689
* @stability experimental
16901690
*/
1691-
export interface IRoleRef extends constructs.IConstruct {
1691+
export interface IRoleRef extends constructs.IConstruct, cdk.IEnvironmentAware {
16921692
/**
16931693
* A reference to a Role resource.
16941694
*/
@@ -1841,7 +1841,7 @@ function CfnRolePropsFromCloudFormation(properties: any): cfn_parse.FromCloudFor
18411841
*
18421842
* @stability experimental
18431843
*/
1844-
export interface IFunctionRef extends constructs.IConstruct {
1844+
export interface IFunctionRef extends constructs.IConstruct, cdk.IEnvironmentAware {
18451845
/**
18461846
* A reference to a Function resource.
18471847
*/
@@ -1896,7 +1896,7 @@ export class CfnFunction extends cdk.CfnResource implements cdk.IInspectable, IF
18961896
"properties": props
18971897
});
18981898
1899-
this.roleArn = (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? props.roleArn;
1899+
this.roleArn = (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? cdk.ensureStringOrUndefined(props.roleArn, "roleArn", "iam.IRoleRef | string");
19001900
}
19011901
19021902
public get functionRef(): FunctionReference {

0 commit comments

Comments
 (0)