Open lagroujl opened 7 months ago
I had already created a pull request for this. But I jumped in when I thought this was a little simpler than it turned out to be. So I thought a better approach was to create an issue/socialize the change & implementation.
the main issue is getting around the fact that EvaluateExpression uses SingletonFunction. Which is fine for the existing implementation, but adding the role property can cause some confusing behavior if its not implemented correctly. The first thing I noticed was the UUID is based only on the nodejs version, so you need to also take into account the role passed in, or the first instance of SingletonFunction evaluated will prevent other roles from being used.
Makes sense.
Before we have the fix I guess you probably can find out the IAM role of the lambda SingletonFunction and override it with your existing one. You probably can modify my sample to fit your needs:
While workaround is possible, we still welcome and appreciate any pull requests to address this.
https://github.com/aws/aws-cdk/issues/29212 is also a good example of what can go wrong. With my implementation, if you were to change the role between deployments, any currently running workflows could fail.
I also considered just accepting the first occurrence of EvaluateExpression wins scenario. Then as a user, you have to make sure the role
property is set on every occurrence of EvaluateExpression in the stack.
Another approach could be to just add the uuid to the API of EvaluateExpression. If omitted, EvaluteExpression uses the current behavior, otherwise users can force a uuid without messing with private APIs. It can still land in some non-intuitive behavior but at least the users have a public API & visibility into what the issue is + solves the issue of keeping the same lambda function through configuration updates.
my implemented approach of adding the role's node addr into the uuid, so each role used will create different lambda functions, but reusing a role should also reuse the Lambda function. that only solves this issue and not 29212
@pahud I created this PR: https://github.com/aws/aws-cdk/pull/29288. I'm just curious if I could get some feedback to see if this is the right approach or the CDK even wants to add this feature.
I don't think that your sample is really sufficient. One of the things my customer needs is the ability to use roles that are imported into the stack. Some modification to your sample would let them rename the role that is created, but I don't think it would be possible or that easy to use an imported role.
Describe the feature
EvaluateExpression creates a singleton Lambda function. For customers that requiring strict monitoring of the IAM roles in their AWS accounts, the auto-generated roles may not fit their requirements. This can be alleviated by providing the ability to bring their own role
Use Case
Customers with strict controls over IAM roles that need more precise control of the IAM roles created in their AWS accounts
Proposed Solution
Role
toEvaluateExpressionProps
that takes anIRole
to be used by the SingletonFunctionOther Information
No response
Acknowledgements
CDK version used
2.130.0
Environment details (OS name and version, etc.)
n/a