Skip to content

Conversation

leonmk-aws
Copy link
Contributor

Issue # (if applicable)

Closes #.

Reason for this change

Allow passing L2s and L1s to L1s

Description of changes

Added a new relationship decider to parse the relationship information from the database and allow the other deciders to modify the properties / constructors accordingly. The relationship deciders assumes that:

  • All modules will be built
  • The properties in the relationship will exist (and have the same naming)

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 ?? props.role;
If there were multiple possible IxxRef:
(props.targetArn as SqsIQueueRef)?.queueRef?.queueArn ?? (props.targetArn as SnsITopicRef)?.topicRef?.topicArn ?? props.targetArn

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 flattenCfnIdentityPoolRoleAttachmentRoleMappingProperty(props: CfnIdentityPoolRoleAttachment.RoleMappingProperty): CfnIdentityPoolRoleAttachment.RoleMappingProperty {
  return {
    "ambiguousRoleResolution": props.ambiguousRoleResolution,
    "identityProvider": props.identityProvider,
    "rulesConfiguration": (cdk.isResolvableObject(props.rulesConfiguration) ? props.rulesConfiguration : (props.rulesConfiguration ? flattenCfnIdentityPoolRoleAttachmentRulesConfigurationTypeProperty(props.rulesConfiguration) : undefined)),
    "type": props.type
  };
}

For arrays

 // @ts-ignore TS6133
function flattenCfnCodeSigningConfigAllowedPublishersProperty(props: CfnCodeSigningConfig.AllowedPublishersProperty | cdk.IResolvable): CfnCodeSigningConfig.AllowedPublishersProperty | cdk.IResolvable {
  if (cdk.isResolvableObject(props)) return props;
  return {
    "signingProfileVersionArns": props.signingProfileVersionArns?.map((item: any) => (item as SignerISigningProfileRef)?.signingProfileRef?.signingProfileArn ?? item)
  };
}

For arrays of nested props:

    this.fileSystemConfigs = (cdk.isResolvableObject(props.fileSystemConfigs) ? props.fileSystemConfigs : (props.fileSystemConfigs ? props.fileSystemConfigs.map(flattenCfnFunctionFileSystemConfigProperty) : undefined));

Database assumptions

Properties (the properties in sense of props of a resource but also prop of a type) have a a relationshipRefs array containing the relationship information:
relationshipRefs: [{ typeName: "AWS::IAM::Role", propertyPath: "Arn" }, ...]

Description of how you validated changes

Build works locally, will not pass here because it needs the changes in the database.
Checked the diffs between the previously generated code and the new one.
Added snapshot tests

Checklist


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

@github-actions github-actions bot added the p2 label Oct 10, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team October 10, 2025 08:33
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Oct 10, 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)

@rix0rrr rix0rrr changed the title feat: passing L2s and L1s to L1s feat: L1s can now accept other resources as parameters where relationships are expected Oct 10, 2025
@rix0rrr rix0rrr changed the title feat: L1s can now accept other resources as parameters where relationships are expected feat(core): L1s can now accept other resources as parameters where relationships are expected Oct 10, 2025
@rix0rrr rix0rrr 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 Oct 10, 2025
@rix0rrr rix0rrr changed the title feat(core): L1s can now accept other resources as parameters where relationships are expected feat(core): L1s can now accept constructs as parameters for known resource relationships Oct 10, 2025
@leonmk-aws leonmk-aws added the pr/do-not-merge This PR should not be merged at this time. label Oct 10, 2025
@leonmk-aws leonmk-aws force-pushed the leonmk-harmony branch 2 times, most recently from 624f91c to 7ec3cc9 Compare October 13, 2025 07:33
@leonmk-aws leonmk-aws marked this pull request as ready for review October 13, 2025 08:36
@rix0rrr rix0rrr changed the title feat(core): L1s can now accept constructs as parameters for known resource relationships feat(core): Cfn constructs (L1s) can now accept constructs as parameters for known resource relationships Oct 14, 2025
@rix0rrr rix0rrr changed the title feat(core): Cfn constructs (L1s) can now accept constructs as parameters for known resource relationships feat(core): cfn constructs (L1s) can now accept constructs as parameters for known resource relationships Oct 14, 2025
@aws-cdk-automation aws-cdk-automation dismissed their stale review October 14, 2025 08:49

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

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.

Add a unit tests somewhere that looks like this:

test('it works', () => {
  const stack = new Stack();

  const cfnRole = new CfnRole(stack, ...);
  const cfnFunction = new CfnFunction(stack, ..., {
    role: cfnRole,    // <-- I want to see this compile
  });

  Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Function', { 
    Role: { Ref: ... }    // <-- and this produce the right template
  });
});

Can be any two resources that show up in the relationship data, pick your fave.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS. p2 pr/do-not-merge This PR should not be merged at this time. 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.

3 participants