-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(lambda): enable RuntimeManagementConfig #23891
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.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
✅ Updated pull request passes all PRLinter validations. Dissmissing 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.
Thanks for the PR, this looks pretty good. I'd prefer to keep this interface as flat props, since in most cases only one will need to be specified (I'm figuring most customers will prefer auto
for runtime management)
export interface RuntimeManagement { | ||
/** | ||
* The ARN of the runtime version you want the function to use. | ||
* | ||
* Note. | ||
* This is only required if you're using the Manual runtime update mode. | ||
* | ||
* @default none | ||
*/ | ||
readonly arn?: string; | ||
/** | ||
* Specify the runtime update mode. | ||
*/ | ||
readonly mode: UpdateRuntimeOn; | ||
} |
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.
Can we remove this interface, and just have runtimeManagementVersionArn
and runtimeManagementMode
as props?
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.
I changed this to flat.
if (props.runtimeManagement) { | ||
if (props.runtimeManagement.mode === UpdateRuntimeOn.MANUAL) { | ||
rmc = { | ||
runtimeVersionArn: props.runtimeManagement.arn, |
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.
we need to throw an error if runtimeManagement.arn
is not defined
* Sets the runtime management configuration for a function's version. | ||
* @default Auto | ||
*/ | ||
readonly runtimeManagement?: RuntimeManagement; |
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.
readonly runtimeManagement?: RuntimeManagement; | |
readonly runtimeManagementMode?: RuntimeManagementMode; | |
readonly runtimeManagementVersionArn?: string; |
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.
Implemented as specified in .manual(arn)
of RuntimeManagementMode.
Co-authored-by: Calvin Combs <[email protected]>
Co-authored-by: Calvin Combs <[email protected]>
Co-authored-by: Calvin Combs <[email protected]>
Pull request has been modified.
Thank you. I have applied the patches I could, but I will try again. runtimeManagementMode: UpdateRuntimeOn.MANUAL_ARN(<someArn>) |
why |
That is a fair point. I was using the original parameter names as they are, but I will consider a proposal to revise the naming to match. https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-lambda-function-runtimemanagementconfig.html#cfn- lambda-function-runtimemanagementconfig-updateruntimeon |
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.
nice work! lgtm
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). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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). |
Introducing AWS Lambda runtime management controls
https://aws.amazon.com/jp/blogs/compute/introducing-aws-lambda-runtime-management-controls/
This setting achieves the following set values.
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-function.html#cfn-lambda-functionruntimemanagementconfig
I have not been able to test this CFn as it does not seem to be supported by cdk. It's only a design.
Closes #23890.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license