Skip to content

Commit f238629

Browse files
mrgrainmrgrain
authored
feat(mixins-preview): allow passing resource objects into properties in CFN Property mixins (#37148)
### Issue # (if applicable) N/A ### Reason for this change The spec2mixins code generator was not resolving deeply nested property types. When a CloudFormation resource has properties with complex nested structures that contain relationship references (e.g. a KMS key ID inside an encryption configuration), the generated mixin code would not include these nested types or their relationship-aware flatten functions. This meant that mixin users could not pass L2 construct references like `kms.Key` as deeply nested properties — they would need to manually resolve ARNs instead. ### Description of changes The `RelationshipDecider` in the mixin builder had both `enableRelationships` and `enableNestedRelationships` set to `false`. Enabling them allows the code generator to resolve relationship references within nested property types, generating the appropriate flatten functions and ref-aware property types. Enabling this revealed two issues that needed fixing: First, the `PartialTypeDefinitionStruct` (used by mixins) forces all properties to be optional, but the generated nested flatten functions assumed the original required/optional status from the CloudFormation spec. This caused TypeScript compilation errors where `undefined` values were passed to flatten functions that didn't accept them. To fix this, `TypeDefinitionStruct.build()` now delegates resolver expression generation to an overridable `resolverExpression()` method. The `PartialTypeDefinitionStruct` overrides this to wrap all flatten function calls with an undefined guard. Second, the top-level mixin constructor needed a flatten function to resolve relationship references before storing props. A standalone `flattenCfnXxxMixinProps()` function is now generated for each mixin that has resolvable properties. This function builds a sparse result object — it only adds keys for properties that were actually set (not `undefined`). This is critical because the `deepMerge` utility checks `key in source`, so returning `{ tags: undefined }` would overwrite the construct's existing `TagManager` with `undefined`. The constructor then spreads both the original props and the resolved overrides: `this.props = { ...props, ...flattenFn(props) }`. ### Describe any new or updated permissions being added N/A ### Description of how you validated changes Added a codegen snapshot test that exercises the deeply nested relationship code path by creating a resource with a nested `EncryptionConfig` type containing a relationship ref to another resource. The snapshot verifies the generated flatten function correctly guards all properties against `undefined` and only includes defined keys in the result. Added an S3 usage test that passes a `kms.Key` construct directly as `kmsMasterKeyId` in a deeply nested bucket encryption configuration and asserts that the synthesized template contains `Fn::GetAtt` for the key ARN. Full test suite passes (172 tests, 9 integration tests unchanged). ### 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 97d3ed4 commit f238629

7 files changed

Lines changed: 346 additions & 16 deletions

File tree

‎packages/@aws-cdk/mixins-preview/README.md‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,6 @@ distribution
148148
Configures vended logs delivery for supported resources when a pre-created destination is provided:
149149

150150
```typescript
151-
import '@aws-cdk/mixins-preview/with';
152151
import * as cloudfrontMixins from '@aws-cdk/mixins-preview/aws-cloudfront/mixins';
153152

154153
// Create CloudFront distribution
@@ -180,7 +179,6 @@ distribution
180179
Vended Logs Configuration for Cross Account delivery (only supported for S3 and Firehose destinations)
181180

182181
```typescript
183-
import '@aws-cdk/mixins-preview/with';
184182
import * as logDestinations from '@aws-cdk/mixins-preview/aws-logs';
185183
import * as cloudfrontMixins from '@aws-cdk/mixins-preview/aws-cloudfront/mixins';
186184

@@ -230,8 +228,6 @@ distribution
230228
For every CloudFormation resource, CDK Mixins automatically generates type-safe property mixins. These allow you to apply L1 properties with full TypeScript support:
231229

232230
```typescript
233-
import '@aws-cdk/mixins-preview/with';
234-
235231
new s3.Bucket(scope, "Bucket")
236232
.with(new CfnBucketPropsMixin({
237233
versioningConfiguration: { status: "Enabled" },
@@ -242,6 +238,24 @@ new s3.Bucket(scope, "Bucket")
242238
}));
243239
```
244240

241+
Deeply nested properties support cross-service references, such as passing a KMS key for encryption:
242+
243+
```typescript
244+
const key = new kms.Key(scope, "Key");
245+
246+
new s3.Bucket(scope, "Bucket")
247+
.with(new CfnBucketPropsMixin({
248+
bucketEncryption: {
249+
serverSideEncryptionConfiguration: [{
250+
serverSideEncryptionByDefault: {
251+
sseAlgorithm: "aws:kms",
252+
kmsMasterKeyId: key,
253+
},
254+
}],
255+
},
256+
}));
257+
```
258+
245259
Property mixins support two merge strategies:
246260

247261
```typescript

‎packages/@aws-cdk/mixins-preview/rosetta/default.ts-fixture‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as dynamodb from 'aws-cdk-lib/aws-dynamodb';
77
import * as cloudfront from 'aws-cdk-lib/aws-cloudfront';
88
import * as origins from 'aws-cdk-lib/aws-cloudfront-origins';
99
import * as iam from 'aws-cdk-lib/aws-iam';
10+
import * as kms from 'aws-cdk-lib/aws-kms';
1011
import '@aws-cdk/mixins-preview/with';
1112
import { Mixins, Mixin, IConstructSelector } from 'aws-cdk-lib/core';
1213
import { IMixin } from 'constructs';

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

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ import { CDK_CORE, CONSTRUCTS } from '@aws-cdk/spec2cdk/lib/cdk/cdk';
44
import { ResourceDecider } from '@aws-cdk/spec2cdk/lib/cdk/resource-decider';
55
import { TypeConverter } from '@aws-cdk/spec2cdk/lib/cdk/type-converter';
66
import { RelationshipDecider } from '@aws-cdk/spec2cdk/lib/cdk/relationship-decider';
7-
import type { Method } from '@cdklabs/typewriter';
8-
import { ExternalModule, Module, ClassType, Stability, StructType, Type, expr, stmt, $T, ThingSymbol, $this, CallableProxy } from '@cdklabs/typewriter';
7+
import { ResolverBuilder } from '@aws-cdk/spec2cdk/lib/cdk/resolver-builder';
8+
import type { Expression, Method } from '@cdklabs/typewriter';
9+
import { ExternalModule, FreeFunction, Module, ClassType, Stability, StructType, Type, expr, stmt, $T, ThingSymbol, $this, CallableProxy } from '@cdklabs/typewriter';
910
import { MIXINS_COMMON, MIXINS_UTILS } from './helpers';
1011
import type { AddServiceProps, LibraryBuilderProps } from '@aws-cdk/spec2cdk/lib/cdk/library-builder';
1112
import { LibraryBuilder } from '@aws-cdk/spec2cdk/lib/cdk/library-builder';
@@ -123,8 +124,8 @@ class L1PropsMixin extends ClassType {
123124
});
124125

125126
this.relationshipDecider = new RelationshipDecider(this.resource, db, {
126-
enableRelationships: false,
127-
enableNestedRelationships: false,
127+
enableRelationships: true,
128+
enableNestedRelationships: true,
128129
refsImportLocation: 'aws-cdk-lib/interfaces',
129130
});
130131
this.converter = TypeConverter.forMixin({
@@ -140,23 +141,60 @@ class L1PropsMixin extends ClassType {
140141
* Build the elements of the L1PropsMixin Class and types
141142
*/
142143
public build() {
144+
const resolverBuilder = new ResolverBuilder(this.converter, this.relationshipDecider, this.scope);
145+
143146
// Build the props type with all properties optional
147+
let needsFlatten = false;
148+
const flattenResolvers: Array<{ name: string; resolver: (props: Expression) => Expression }> = [];
144149
for (const prop of this.decider.propsProperties) {
145-
if (prop.propertySpec.type.fqn) {
146-
continue;
147-
}
148150
this.propsType.addProperty({
149151
...prop.propertySpec,
150152
optional: true,
151153
});
154+
155+
const specProp = this.resource.properties[prop.cfnMapping.cfnName];
156+
// Mark as required so buildResolver skips the inline undefined guard;
157+
// the top-level flatten function already has an if-guard for each property.
158+
const result = resolverBuilder.buildResolver({ ...specProp, required: true }, prop.cfnMapping.cfnName);
159+
const hasRelationships = specProp.relationshipRefs && specProp.relationshipRefs.length > 0;
160+
const hasNestedFlatten = this.relationshipDecider.needsFlatteningFunction(prop.cfnMapping.cfnName, specProp);
161+
if (hasRelationships || hasNestedFlatten) {
162+
needsFlatten = true;
163+
flattenResolvers.push({
164+
name: result.name,
165+
// The outer if-guard in the flatten function body already ensures the value is defined,
166+
// so no additional undefined check is needed here.
167+
resolver: result.resolver,
168+
});
169+
}
170+
}
171+
172+
// Generate a top-level flatten function if any props need resolving
173+
let flattenFunction: FreeFunction | undefined;
174+
if (needsFlatten) {
175+
flattenFunction = new FreeFunction(this.scope, {
176+
name: naming.flattenFunctionNameFromType(this.propsType),
177+
returnType: this.propsType.type,
178+
parameters: [{ name: 'props', type: this.propsType.type }],
179+
});
180+
const fnProps = flattenFunction.parameters[0];
181+
const ret = expr.ident('ret');
182+
flattenFunction.addBody(
183+
stmt.constVar(expr.directCode('ret: any'), expr.object()),
184+
...flattenResolvers.map(r =>
185+
stmt.if_(expr.binOp(expr.get(fnProps, r.name), '!==', expr.UNDEFINED))
186+
.then(stmt.assign(expr.get(ret, r.name), r.resolver(fnProps))),
187+
),
188+
stmt.ret(ret),
189+
);
152190
}
153191

154-
this.makeConstructor();
192+
this.makeConstructor(flattenFunction);
155193
const supports = this.makeSupportsMethod();
156194
this.makeApplyToMethod(supports);
157195
}
158196

159-
private makeConstructor() {
197+
private makeConstructor(flattenFunction?: FreeFunction) {
160198
const optionsType = MIXINS_COMMON.CfnPropertyMixinOptions;
161199

162200
this.addProperty({
@@ -204,7 +242,9 @@ class L1PropsMixin extends ClassType {
204242

205243
init.addBody(
206244
expr.sym(new ThingSymbol('super', this.scope)).call(),
207-
stmt.assign($this.props, props),
245+
stmt.assign($this.props, flattenFunction
246+
? expr.object(expr.splat(props), expr.splat(CallableProxy.fromName(flattenFunction.name, this.scope).invoke(props)))
247+
: props),
208248
stmt.assign($this.strategy, expr.binOp(options?.prop('strategy'), '??', MIXINS_COMMON.PropertyMergeStrategy.MERGE)),
209249
);
210250
}

‎packages/@aws-cdk/mixins-preview/test/codegen/__snapshots__/resources.test.ts.snap‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,116 @@ export interface CfnThingMixinProps {
101101
readonly id?: string;
102102
}"
103103
`;
104+
105+
exports[`L1 property mixin with deeply nested relationship properties 1`] = `
106+
"/* eslint-disable prettier/prettier, @stylistic/max-len */
107+
import * as cdk from "aws-cdk-lib/core";
108+
import * as constructs from "constructs";
109+
import * as mixins from "../../mixins";
110+
import * as helpers from "../../util/property-mixins";
111+
import * as service from "aws-cdk-lib/aws-some";
112+
import { aws_other as otherRefs } from "aws-cdk-lib/interfaces";
113+
114+
/**
115+
* @cloudformationResource AWS::Some::Resource
116+
* @mixin true
117+
* @stability external
118+
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-some-resource.html
119+
*/
120+
export class CfnThingPropsMixin extends cdk.Mixin implements constructs.IMixin {
121+
protected static readonly CFN_PROPERTY_KEYS: Array<string> = ["encryption", "id"];
122+
123+
protected readonly props: CfnThingMixinProps;
124+
125+
protected readonly strategy: mixins.PropertyMergeStrategy;
126+
127+
/**
128+
* Create a mixin to apply properties to \`AWS::Some::Resource\`.
129+
*
130+
* @param props L1 properties to apply
131+
* @param options Mixin options
132+
*/
133+
public constructor(props: CfnThingMixinProps, options: mixins.CfnPropertyMixinOptions = {}) {
134+
super();
135+
this.props = {
136+
...props,
137+
...flattenCfnThingMixinProps(props)
138+
};
139+
this.strategy = (options.strategy ?? mixins.PropertyMergeStrategy.MERGE);
140+
}
141+
142+
/**
143+
* Check if this mixin supports the given construct
144+
*/
145+
public supports(construct: constructs.IConstruct): construct is service.CfnThing {
146+
return (cdk.CfnResource.isCfnResource(construct) && service.CfnThing.isCfnThing(construct));
147+
}
148+
149+
/**
150+
* Apply the mixin properties to the construct
151+
*/
152+
public applyTo(construct: constructs.IConstruct): void {
153+
if (this.supports(construct)) {
154+
if ((this.strategy === mixins.PropertyMergeStrategy.MERGE)) {
155+
helpers.deepMerge(construct, this.props, CfnThingPropsMixin.CFN_PROPERTY_KEYS);
156+
} else {
157+
helpers.shallowAssign(construct, this.props, CfnThingPropsMixin.CFN_PROPERTY_KEYS);
158+
}
159+
}
160+
}
161+
}
162+
163+
export namespace CfnThingPropsMixin {
164+
/**
165+
* @struct
166+
* @stability external
167+
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-some-resource-encryptionconfig.html
168+
*/
169+
export interface EncryptionConfigProperty {
170+
/**
171+
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-some-resource-encryptionconfig.html#cfn-some-resource-encryptionconfig-algorithm
172+
*/
173+
readonly algorithm?: string;
174+
175+
/**
176+
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-some-resource-encryptionconfig.html#cfn-some-resource-encryptionconfig-keyid
177+
*/
178+
readonly keyId?: otherRefs.IKeyRef | string;
179+
}
180+
}
181+
182+
/**
183+
* Properties for CfnThingPropsMixin
184+
*
185+
* @struct
186+
* @stability external
187+
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-some-resource.html
188+
*/
189+
export interface CfnThingMixinProps {
190+
/**
191+
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-some-resource.html#cfn-some-resource-encryption
192+
*/
193+
readonly encryption?: CfnThingPropsMixin.EncryptionConfigProperty | cdk.IResolvable;
194+
195+
/**
196+
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-some-resource.html#cfn-some-resource-id
197+
*/
198+
readonly id?: string;
199+
}
200+
201+
// @ts-ignore TS6133
202+
function flattenCfnThingPropsMixinEncryptionConfigProperty(props: CfnThingPropsMixin.EncryptionConfigProperty | cdk.IResolvable): CfnThingPropsMixin.EncryptionConfigProperty | cdk.IResolvable {
203+
if (cdk.isResolvableObject(props)) return props;
204+
return {
205+
algorithm: (!props.algorithm ? undefined : props.algorithm),
206+
keyId: (!props.keyId ? undefined : cdk.getRefProperty((props.keyId as otherRefs.IKeyRef)?.keyRef, 'keyId') ?? cdk.ensureStringOrUndefined(props.keyId, "keyId", "other.IKeyRef | string"))
207+
};
208+
}
209+
210+
// @ts-ignore TS6133
211+
function flattenCfnThingMixinProps(props: CfnThingMixinProps): CfnThingMixinProps {
212+
const ret: any = {};
213+
if ((props.encryption !== undefined)) ret.encryption = flattenCfnThingPropsMixinEncryptionConfigProperty(props.encryption);
214+
return ret;
215+
}"
216+
`;

‎packages/@aws-cdk/mixins-preview/test/codegen/resources.test.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,74 @@ function moduleForResource(resource: Resource, props: MixinsBuilderProps) {
7878
const info = ast.addResource(resource);
7979
return info.locatedModules[0].module;
8080
}
81+
82+
test('L1 property mixin with deeply nested relationship properties', () => {
83+
// Target resource for the relationship
84+
const targetService = db.allocate('service', {
85+
name: 'aws-other',
86+
shortName: 'other',
87+
capitalized: 'Other',
88+
cloudFormationNamespace: 'AWS::Other',
89+
});
90+
const targetResource = db.allocate('resource', {
91+
name: 'Key',
92+
primaryIdentifier: ['KeyId'],
93+
properties: {
94+
KeyId: {
95+
type: { type: 'string' },
96+
},
97+
},
98+
attributes: {
99+
KeyArn: {
100+
type: { type: 'string' },
101+
},
102+
},
103+
cloudFormationType: 'AWS::Other::Key',
104+
});
105+
db.link('hasResource', targetService, targetResource);
106+
107+
// Nested type with a relationship ref
108+
const nestedType = db.allocate('typeDefinition', {
109+
name: 'EncryptionConfig',
110+
properties: {
111+
KeyId: {
112+
type: { type: 'string' },
113+
relationshipRefs: [{
114+
cloudFormationType: 'AWS::Other::Key',
115+
propertyName: 'KeyId',
116+
}],
117+
},
118+
Algorithm: {
119+
type: { type: 'string' },
120+
},
121+
},
122+
});
123+
124+
const resource = db.allocate('resource', {
125+
name: 'Thing',
126+
primaryIdentifier: ['Id'],
127+
properties: {
128+
Id: {
129+
type: { type: 'string' },
130+
},
131+
Encryption: {
132+
type: { type: 'ref', reference: ref(nestedType) },
133+
},
134+
},
135+
cloudFormationType: 'AWS::Some::Resource',
136+
attributes: {},
137+
});
138+
db.link('hasResource', service, resource);
139+
db.link('usesType', resource, nestedType);
140+
141+
const foundResource = db.lookup('resource', 'cloudFormationType', 'equals', 'AWS::Some::Resource').only();
142+
const module = moduleForResource(foundResource, { db });
143+
const rendered = renderer.render(module);
144+
145+
expect(rendered).toMatchSnapshot();
146+
147+
// Non-relationship props (id) should pass through without flatten wrapping
148+
expect(rendered).not.toContain('ret.id');
149+
// Relationship props (encryption) should be in the flatten function
150+
expect(rendered).toContain('ret.encryption');
151+
});

0 commit comments

Comments
 (0)