Skip to content

Commit 6be7b4b

Browse files
authored
feat(core): cfn constructs (L1s) can now accept constructs as parameters for known resource relationships (#35838)
### Issue # (if applicable) Closes #<issue number here>. ### Reason for this change Reintroduces #35713 ### Description of changes #### This generates code that looks like this for non nested properties: In the properties: `readonly role: IamIRoleRef | string;` This is then used in the constructor: ``` this.role = (props.role as IamIRoleRef)?.roleRef?.roleArn ?? cdk.ensureStringOrUndefined(props.role, "role", "iam.IRoleRef | string"); ``` If there were multiple possible IxxRef: ``` "targetArn": (props.targetArn as SqsIQueueRef)?.queueRef?.queueArn ?? (props.targetArn as SnsITopicRef)?.topicRef?.topicArn ?? cdk.ensureStringOrUndefined(props.targetArn, "targetArn", "sqs.IQueueRef | sns.ITopicRef | string") ``` #### For arrays ``` this.layers = (props.layers?.forEach((item: ILayerVersionRef | string, i: number, arr: Array<ILayerVersionRef | string>) => { arr[i] = (item as ILayerVersionRef)?.layerVersionRef?.layerVersionArn ?? cdk.ensureStringOrUndefined(item, "layers", "lambda.ILayerVersionRef | string"); }), props.layers as Array<string>); ``` ### Nested properties are currently disabled as they are backwards incompatible in the state of this PR #### For nested properties The props behave the same way, "flatten" functions are generated to recursively perform the same role that was done in the constructor for non nested properties: ``` function flattenCfnFunctionCodeProperty(props: CfnFunction.CodeProperty | cdk.IResolvable): CfnFunction.CodeProperty | cdk.IResolvable { if (cdk.isResolvableObject(props)) return props; return { "imageUri": props.imageUri, "s3Bucket": (props.s3Bucket as S3IBucketRef)?.bucketRef?.bucketName ?? cdk.ensureStringOrUndefined(props.s3Bucket, "s3Bucket", "s3.IBucketRef | string"), "s3Key": props.s3Key, "s3ObjectVersion": props.s3ObjectVersion, "sourceKmsKeyArn": props.sourceKmsKeyArn, "zipFile": props.zipFile }; } ``` #### For arrays of nested props: ``` (cdk.isResolvableObject(props.fileSystemConfigs) ? props.fileSystemConfigs : (!props.fileSystemConfigs ? undefined : props.fileSystemConfigs.forEach((item: any, i: number, arr: any[]) => { arr[i] = flattenCfnFunctionFileSystemConfigProperty(item) }), props.fileSystemConfigs)); ``` ### Description of how you validated changes - Checked the diffs between the previously generated code and the new one. - Added snapshot tests - Added unit tests for lambda - Deployed a stack manually consisting of mixes of L1 and L2 resources using the new capabilities this PR adds ### 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 f9a894f commit 6be7b4b

File tree

9 files changed

+1934
-61
lines changed

9 files changed

+1934
-61
lines changed

packages/@aws-cdk/mixins-preview/scripts/spec2mixins/builder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ class L1PropsMixin extends ClassType {
127127
},
128128
});
129129

130-
this.relationshipDecider = new RelationshipDecider(this.resource, db);
130+
this.relationshipDecider = new RelationshipDecider(this.resource, db, false);
131131
this.converter = TypeConverter.forMixin({
132132
db: db,
133133
resource: this.resource,

packages/aws-cdk-lib/aws-lambda/test/function.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5046,6 +5046,10 @@ describe('Lambda Function log group behavior', () => {
50465046
});
50475047

50485048
describe('telemetry metadata', () => {
5049+
afterEach(() => {
5050+
jest.restoreAllMocks();
5051+
});
5052+
50495053
it('redaction happens when feature flag is enabled', () => {
50505054
const app = new cdk.App();
50515055
app.node.setContext(cxapi.ENABLE_ADDITIONAL_METADATA_COLLECTION, true);
@@ -5107,6 +5111,104 @@ describe('telemetry metadata', () => {
51075111
expect(fn.node.metadata).toStrictEqual([]);
51085112
});
51095113
});
5114+
describe('L1 Relationships', () => {
5115+
it('simple union', () => {
5116+
const stack = new cdk.Stack();
5117+
const role = new iam.Role(stack, 'SomeRole', {
5118+
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
5119+
});
5120+
new lambda.CfnFunction(stack, 'MyLambda', {
5121+
code: { zipFile: 'foo' },
5122+
role: role, // Simple Union
5123+
});
5124+
Template.fromStack(stack).hasResource('AWS::Lambda::Function', {
5125+
Properties: {
5126+
Role: { 'Fn::GetAtt': ['SomeRole6DDC54DD', 'Arn'] },
5127+
},
5128+
});
5129+
});
5130+
5131+
it('array of unions', () => {
5132+
const stack = new cdk.Stack();
5133+
const role = new iam.Role(stack, 'SomeRole', {
5134+
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
5135+
});
5136+
const layer1 = new lambda.LayerVersion(stack, 'LayerVersion1', {
5137+
code: lambda.Code.fromAsset(path.join(__dirname, 'my-lambda-handler')),
5138+
compatibleRuntimes: [lambda.Runtime.PYTHON_3_13],
5139+
});
5140+
const layer2 = new lambda.LayerVersion(stack, 'LayerVersion2', {
5141+
code: lambda.Code.fromAsset(path.join(__dirname, 'my-lambda-handler')),
5142+
compatibleRuntimes: [lambda.Runtime.PYTHON_3_13],
5143+
});
5144+
new lambda.CfnFunction(stack, 'MyLambda', {
5145+
code: { zipFile: 'foo' },
5146+
role: role,
5147+
layers: [layer1, layer2], // Array of Unions
5148+
});
5149+
Template.fromStack(stack).hasResource('AWS::Lambda::Function', {
5150+
Properties: {
5151+
Role: { 'Fn::GetAtt': ['SomeRole6DDC54DD', 'Arn'] },
5152+
Layers: [{ Ref: 'LayerVersion139D4D7A8' }, { Ref: 'LayerVersion23E5F3CEA' }],
5153+
},
5154+
});
5155+
});
5156+
5157+
it('array reference should be valid', () => {
5158+
const stack = new cdk.Stack();
5159+
const role = new iam.Role(stack, 'SomeRole', {
5160+
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
5161+
});
5162+
const layer1 = new lambda.LayerVersion(stack, 'LayerVersion1', {
5163+
code: lambda.Code.fromAsset(path.join(__dirname, 'my-lambda-handler')),
5164+
compatibleRuntimes: [lambda.Runtime.PYTHON_3_13],
5165+
});
5166+
const layerArray = [layer1, 'layer2Arn'];
5167+
new lambda.CfnFunction(stack, 'MyLambda', {
5168+
code: { zipFile: 'foo' },
5169+
role: role,
5170+
layers: layerArray,
5171+
});
5172+
5173+
layerArray.push('layer3Arn');
5174+
5175+
Template.fromStack(stack).hasResource('AWS::Lambda::Function', {
5176+
Properties: {
5177+
Role: { 'Fn::GetAtt': ['SomeRole6DDC54DD', 'Arn'] },
5178+
Layers: [{ Ref: 'LayerVersion139D4D7A8' }, 'layer2Arn', 'layer3Arn'],
5179+
},
5180+
});
5181+
});
5182+
5183+
it('tokens should be passed as is', () => {
5184+
const stack = new cdk.Stack();
5185+
const role = new iam.Role(stack, 'SomeRole', {
5186+
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
5187+
});
5188+
const bucket = new s3.Bucket(stack, 'MyBucket');
5189+
5190+
const codeToken = cdk.Token.asAny({
5191+
resolve: () => ({ s3Bucket: bucket.bucketName }),
5192+
});
5193+
5194+
const fsConfigToken = cdk.Token.asAny({
5195+
resolve: () => ([{ arn: 'TestArn', localMountPath: '/mnt' }]),
5196+
});
5197+
5198+
new lambda.CfnFunction(stack, 'MyLambda', {
5199+
code: codeToken,
5200+
role: role,
5201+
fileSystemConfigs: fsConfigToken,
5202+
});
5203+
Template.fromStack(stack).hasResource('AWS::Lambda::Function', {
5204+
Properties: {
5205+
Role: { 'Fn::GetAtt': ['SomeRole6DDC54DD', 'Arn'] },
5206+
Code: { S3Bucket: { Ref: 'MyBucketF68F3FF0' } },
5207+
FileSystemConfigs: [{ Arn: 'TestArn', LocalMountPath: '/mnt' }],
5208+
},
5209+
});
5210+
});
5211+
});
51105212

51115213
function newTestLambda(scope: constructs.Construct) {
51125214
return new lambda.Function(scope, 'MyLambda', {

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,6 @@ export class AwsCdkLibBuilder extends LibraryBuilder<AwsCdkLibServiceSubmodule>
182182
submodule.registerResource(resource.cloudFormationType, resourceClass);
183183
submodule.registerSelectiveImports(...resourceClass.imports);
184184
submodule.augmentations?.module.augmentResource(resource, resourceClass);
185-
186-
for (const selectiveImport of submodule.imports) {
187-
const sourceModule = new Module(selectiveImport.moduleName);
188-
sourceModule.importSelective(submodule.resourcesMod.module, selectiveImport.types.map((t) => `${t.originalType} as ${t.aliasedType}`), {
189-
fromLocation: relativeImportPath(submodule.resourcesMod.filePath, sourceModule.name),
190-
});
191-
}
192185
}
193186

194187
private createResourceModule(moduleName: string, service: Service): LocatedModule<Module> {
@@ -262,7 +255,15 @@ export class AwsCdkLibBuilder extends LibraryBuilder<AwsCdkLibServiceSubmodule>
262255
grantModule.build(Object.fromEntries(submodule.resources), props?.nameSuffix);
263256
}
264257

265-
super.postprocessSubmodule(submodule);
258+
// Apply selective imports only to resources module
259+
for (const selectiveImport of submodule.imports) {
260+
const sourceModule = new Module(selectiveImport.moduleName);
261+
sourceModule.importSelective(
262+
submodule.resourcesMod.module,
263+
selectiveImport.types.map((t) => `${t.originalType} as ${t.aliasedType}`),
264+
{ fromLocation: relativeImportPath(submodule.resourcesMod, sourceModule.name) },
265+
);
266+
}
266267

267268
// Add an import for the interfaces file to the entry point file (make sure not to do it twice)
268269
if (!submodule.interfaces?.module.isEmpty() && this.interfacesEntry && submodule.didCreateInterfaceModule) {

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,19 @@ import { getReferenceProps } from './reference-props';
55
import { log } from '../util';
66
import { SelectiveImport } from './service-submodule';
77

8-
// For now we want relationships to be applied only for these services
9-
export const RELATIONSHIP_SERVICES: string[] = [];
8+
/**
9+
* We currently disable the relationship on the properties of types because they would create a backwards incompatible change
10+
* by broadening the output type as types are used both in input and output. This represents:
11+
* Relationship counts:
12+
* Resource-level (non-nested): 598
13+
* Type-level (nested): 483 <- these are disabled by this flag
14+
* Total: 1081
15+
* Properties with relationships:
16+
* Resource-level (non-nested): 493
17+
* Type-level (nested): 358
18+
* Total: 851
19+
*/
20+
export const GENERATE_RELATIONSHIPS_ON_TYPES = false;
1021

1122
/**
1223
* Represents a cross-service property relationship that enables references
@@ -30,7 +41,7 @@ export class RelationshipDecider {
3041
private readonly namespace: string;
3142
public readonly imports = new Array<SelectiveImport>();
3243

33-
constructor(readonly resource: Resource, private readonly db: SpecDatabase) {
44+
constructor(readonly resource: Resource, private readonly db: SpecDatabase, private readonly enableRelationships = true) {
3445
this.namespace = namespaceFromResource(resource);
3546
}
3647

@@ -61,7 +72,7 @@ export class RelationshipDecider {
6172
* properties as a relationship can only target a primary identifier or arn
6273
*/
6374
private findTargetResource(sourcePropName: string, relationship: RelationshipRef) {
64-
if (!RELATIONSHIP_SERVICES.some(s => this.resource.cloudFormationType.toLowerCase().startsWith(`aws::${s}::`))) {
75+
if (!this.enableRelationships) {
6576
return undefined;
6677
}
6778
const targetResource = this.db.lookup('resource', 'cloudFormationType', 'equals', relationship.cloudFormationType).only();
@@ -135,6 +146,9 @@ export class RelationshipDecider {
135146
* Checks if a given property needs a flattening function or not
136147
*/
137148
public needsFlatteningFunction(propName: string, prop: Property, visited = new Set<string>()): boolean {
149+
if (!GENERATE_RELATIONSHIPS_ON_TYPES) {
150+
return false;
151+
}
138152
if (this.hasValidRelationships(propName, prop.relationshipRefs)) {
139153
return true;
140154
}

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { DefinitionReference, Property } from '@aws-cdk/service-spec-types';
22
import { expr, Expression, Module, Type } from '@cdklabs/typewriter';
33
import { CDK_CORE } from './cdk';
4-
import { RelationshipDecider, Relationship } from './relationship-decider';
4+
import { RelationshipDecider, Relationship, GENERATE_RELATIONSHIPS_ON_TYPES } from './relationship-decider';
55
import { NON_RESOLVABLE_PROPERTY_NAMES } from './tagging';
66
import { TypeConverter } from './type-converter';
77
import { flattenFunctionNameFromType, propertyNameFromCloudFormation } from '../naming';
@@ -28,7 +28,8 @@ export class ResolverBuilder {
2828
private readonly module: Module,
2929
) {}
3030

31-
public buildResolver(prop: Property, cfnName: string): ResolverResult {
31+
public buildResolver(prop: Property, cfnName: string, isTypeProp = false): ResolverResult {
32+
const shouldGenerateRelationships = isTypeProp ? GENERATE_RELATIONSHIPS_ON_TYPES : true;
3233
const name = propertyNameFromCloudFormation(cfnName);
3334
const baseType = this.converter.typeFromProperty(prop);
3435

@@ -37,17 +38,19 @@ export class ResolverBuilder {
3738
// but didn't.
3839
const resolvableType = cfnName in NON_RESOLVABLE_PROPERTY_NAMES ? baseType : this.converter.makeTypeResolvable(baseType);
3940

40-
const relationships = this.relationshipDecider.parseRelationship(name, prop.relationshipRefs);
41-
if (relationships.length > 0) {
42-
return this.buildRelationshipResolver({ relationships, baseType, name, resolvableType });
43-
}
41+
if (shouldGenerateRelationships) {
42+
const relationships = this.relationshipDecider.parseRelationship(name, prop.relationshipRefs);
43+
if (relationships.length > 0) {
44+
return this.buildRelationshipResolver({ relationships, baseType, name, resolvableType });
45+
}
4446

45-
const originalType = this.converter.originalType(baseType);
46-
if (this.relationshipDecider.needsFlatteningFunction(name, prop)) {
47-
const optional = !prop.required;
48-
const typeRef = originalType.type === 'array' ? originalType.element : originalType;
49-
if (typeRef.type === 'ref') {
50-
return this.buildNestedResolver({ name, baseType, typeRef: typeRef, resolvableType, optional });
47+
const originalType = this.converter.originalType(baseType);
48+
if (this.relationshipDecider.needsFlatteningFunction(name, prop)) {
49+
const optional = !prop.required;
50+
const typeRef = originalType.type === 'array' ? originalType.element : originalType;
51+
if (typeRef.type === 'ref') {
52+
return this.buildNestedResolver({ name, baseType, typeRef: typeRef, resolvableType, optional });
53+
}
5154
}
5255
}
5356

@@ -80,8 +83,8 @@ export class ResolverBuilder {
8083
].join(' | ');
8184

8285
// Generates code like:
83-
// For single value: (props.roleArn as IRoleRef)?.roleRef?.roleArn ?? (props.roleArn as IUserRef)?.userRef?.userArn ?? props.roleArn
84-
// For array: props.roleArns?.map((item: any) => (item as IRoleRef)?.roleRef?.roleArn ?? (item as IUserRef)?.userRef?.userArn ?? item)
86+
// For single value T | string : (props.xx as IxxxRef)?.xxxRef?.xxxArn ?? cdk.ensureStringOrUndefined(props.xxx, "xxx", "iam.IxxxRef | string");
87+
// For array <T | string>[]: (props.xx?.forEach((item: T | string, i: number, arr: Array<T | string>) => { arr[i] = (item as T)?.xxxRef?.xx ?? cdk.ensureStringOrUndefined(item, "xxx", "lambda.T | string"); }), props.xxx as Array<string>);
8588
// Ensures that arn properties always appear first in the chain as they are more general
8689
const arnRels = relationships.filter(r => r.propName.toLowerCase().endsWith('arn'));
8790
const otherRels = relationships.filter(r => !r.propName.toLowerCase().endsWith('arn'));
@@ -93,7 +96,9 @@ export class ResolverBuilder {
9396
].join(' ?? ');
9497
const resolver = (_: Expression) => {
9598
if (resolvableType.arrayOfType) {
96-
return expr.directCode(`props.${name}?.map((item: any) => ${ buildChain('item') })`);
99+
return expr.directCode(
100+
`(props.${name}?.forEach((item: ${propType.arrayOfType!.toString()}, i: number, arr: ${propType.toString()}) => { arr[i] = ${buildChain('item')}; }), props.${name} as ${resolvableType.toString()})`,
101+
);
97102
} else {
98103
return expr.directCode(buildChain(`props.${name}`));
99104
}
@@ -118,11 +123,11 @@ export class ResolverBuilder {
118123
const isArray = baseType.arrayOfType !== undefined;
119124

120125
const flattenCall = isArray
121-
? propValue.callMethod('map', expr.ident(functionName))
126+
? expr.directCode(`props.${name}.forEach((item: any, i: number, arr: any[]) => { arr[i] = ${functionName}(item) }), props.${name}`)
122127
: expr.ident(functionName).call(propValue);
123128

124129
const condition = optional
125-
? expr.cond(propValue).then(flattenCall).else(expr.UNDEFINED)
130+
? expr.cond(expr.not(propValue)).then(expr.UNDEFINED).else(flattenCall)
126131
: flattenCall;
127132

128133
return isArray

tools/@aws-cdk/spec2cdk/lib/cdk/service-submodule.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export class BaseServiceSubmodule {
106106
const existingModuleImport = this.findSelectiveImport(theImport);
107107
if (!existingModuleImport) {
108108
this.selectiveImports.push(theImport);
109-
return;
109+
continue;
110110
}
111111

112112
// We need to avoid importing the same reference multiple times

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class TypeDefinitionDecider {
3838
private handlePropertyDefault(cfnName: string, prop: Property) {
3939
const optional = !prop.required;
4040

41-
const resolverResult = this.resolverBuilder.buildResolver(prop, cfnName);
41+
const resolverResult = this.resolverBuilder.buildResolver(prop, cfnName, true);
4242

4343
this.properties.push({
4444
propertySpec: {

0 commit comments

Comments
 (0)