Skip to content

Conversation

@leonmk-aws
Copy link
Contributor

@leonmk-aws leonmk-aws commented Oct 24, 2025

Issue # (if applicable)

Closes #.

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


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@leonmk-aws leonmk-aws added the pr/do-not-merge This PR should not be merged at this time. label Oct 24, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team October 24, 2025 12:12
@github-actions github-actions bot added the p2 label Oct 24, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Oct 24, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This review is outdated)

@leonmk-aws leonmk-aws added the pr/request-cli-integ-tests Request CLI integ tests to be run. You will need to review the code and approve the deployment. label Oct 24, 2025
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there something we can do in the codegen to not change the identity of the input array, so we don't have to go down all the places where our change has affected the L2s?

Comment on lines 5062 to 5064
afterEach(() => {
getPrototypeOfSpy.mockRestore();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably jest.restoreAll() in the beforeEach?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a restore in afterEach: a mock for one test should not affect the others

@rix0rrr rix0rrr changed the title feat(core): cfn constructs (L1s) can now accept constructs as parameters for known resource relationships (#35713) feat(core): cfn constructs (L1s) can now accept constructs as parameters for known resource relationships Oct 27, 2025
@leonmk-aws leonmk-aws requested a review from a team as a code owner November 6, 2025 12:22
@leonmk-aws
Copy link
Contributor Author

leonmk-aws commented Nov 7, 2025

The codegen has been updated to modify the arrays in place rather than creating new ones (see description)

@aws-cdk-automation
Copy link
Collaborator

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

@leonmk-aws leonmk-aws added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Nov 20, 2025
@aws-cdk-automation aws-cdk-automation dismissed their stale review November 20, 2025 14:03

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@leonmk-aws leonmk-aws added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Nov 20, 2025
@gasolima gasolima removed the pr/do-not-merge This PR should not be merged at this time. label Nov 20, 2025
@mergify
Copy link
Contributor

mergify bot commented Nov 20, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify
Copy link
Contributor

mergify bot commented Nov 20, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 6be7b4b into aws:main Nov 20, 2025
17 of 19 checks passed
@github-actions
Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

contribution/core This is a PR that came from AWS. p2 pr/request-cli-integ-tests Request CLI integ tests to be run. You will need to review the code and approve the deployment. pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants