-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(core): cfn constructs (L1s) can now accept constructs as parameters for known resource relationships #35713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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)
624f91c
to
7ec3cc9
Compare
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
There was a problem hiding this 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.
7ec3cc9
to
e084b83
Compare
e084b83
to
77f8e05
Compare
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:
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:
For arrays
For arrays of nested props:
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