Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(cli): support hotswapping Lambda functions with inline code #18408

Merged
merged 11 commits into from
Jan 19, 2022
Merged

feat(cli): support hotswapping Lambda functions with inline code #18408

merged 11 commits into from
Jan 19, 2022

Conversation

tmokmss
Copy link
Contributor

@tmokmss tmokmss commented Jan 13, 2022

Similarly to #18319, this PR supports hotswap of Lambda functions that use InlineCode.

InlineCode uses CloudFormation ZipFile feature. To emulate its behavior, we create a zip file of the provided inline code with its filename index.js or index.py according to the runtime (CFn only supports python or nodejs for ZipFile), and pass the file's binary buffer to Lambda API.

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

@gitpod-io
Copy link

gitpod-io bot commented Jan 13, 2022

@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jan 13, 2022
@tmokmss tmokmss mentioned this pull request Jan 13, 2022
Merged
@comcalvi comcalvi assigned skinny85 and comcalvi and unassigned rix0rrr Jan 13, 2022
comcalvi
comcalvi previously requested changes Jan 13, 2022
View reviewed changes
Copy link
Contributor

@comcalvi comcalvi left a comment •
edited

Choose a reason for hiding this comment

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

Awesome work! I have a few comments, @skinny85 let me know what you think. Also, @tmokmss can you update the README to please the linter?

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Show resolved Hide resolved
packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts Show resolved Hide resolved
packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts Show resolved Hide resolved
packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated Show resolved Hide resolved
skinny85
skinny85 previously requested changes Jan 14, 2022
View reviewed changes
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Looks great @tmokmss! A few minor comments before we merge this in 🙂.

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated Show resolved Hide resolved
packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated
// We must create a zip package containing a file with the inline code
const rawZipFile = await evaluateCfnTemplate.evaluateCfnExpression(updatedProp.newValue[newPropName]);
// file extension must be chosen depending on the runtime
const runtime: string = await evaluateCfnTemplate.evaluateCfnExpression(change.newValue.Properties!.Runtime);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the ! really required? Can we go without it?

Suggested change
const runtime: string = await evaluateCfnTemplate.evaluateCfnExpression(change.newValue.Properties!.Runtime);
const runtime: string = await evaluateCfnTemplate.evaluateCfnExpression(change.newValue.Properties.Runtime);

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, let's call this variable functionRuntime, just to make it's very clear what runtime we are talking about here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Properties is possibly undefined so we need to care it.

aws-cdk/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts

Lines 475 to 480 in c7c51de

export interface Resource {
Type: string;
Properties?: { [name: string]: any };
[key: string]: any;
}

I replaced ! to ? to care when runtime is undefined, c.f. s3-bucket-deployments.ts (idk if there's such a case though)

aws-cdk/packages/aws-cdk/lib/api/hotswap/s3-bucket-deployments.ts

Lines 25 to 28 in c7c51de

const functionName = await evaluateCfnTemplate.evaluateCfnExpression(change.newValue.Properties?.ServiceToken);
if (!functionName) {
return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT;
}

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated Show resolved Hide resolved
packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated Show resolved Hide resolved
packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated Show resolved Hide resolved
packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated Show resolved Hide resolved
packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Outdated Show resolved Hide resolved
packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts Outdated Show resolved Hide resolved
packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts Show resolved Hide resolved
packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Show resolved Hide resolved
@mergify mergify bot dismissed stale reviews from comcalvi and skinny85 January 14, 2022 01:30

Pull request has been modified.

@tmokmss
Copy link
Contributor Author

tmokmss commented Jan 14, 2022

Thank you very much @comcalvi @skinny85! I fixed all of the review points. I'll fix possible conflicts after #18319 will be merged.

mergify bot pushed a commit that referenced this pull request Jan 14, 2022
@tmokmss
feat(cli): support hotswapping Lambda functions that use Docker images (
6b553b7 
#18319)

closes #18302

We must just update `ImageUri` with the new ECR image url.

PR for `InlineCode` hotswap: #18408
 
`----`

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@tmokmss
Copy link
Contributor Author

tmokmss commented Jan 15, 2022

@skinny85
I resolved conflicts with master branch. Now ready for merge:)

skinny85
skinny85 approved these changes Jan 18, 2022
View reviewed changes
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Looks great @tmokmss, thanks for the contribution!

@skinny85 skinny85 changed the title feat(cli): Support hotswap for Lambda code with InlineCode feat(cli): support hotswapping Lambda functions with inline code Jan 18, 2022
@mergify
Copy link
Contributor

mergify bot commented Jan 18, 2022

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: fb4656c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit d0b8512 into aws:master Jan 19, 2022
@mergify
Copy link
Contributor

mergify bot commented Jan 19, 2022

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

@tmokmss tmokmss deleted the hotswap_inline_lambda branch January 19, 2022 12:06
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
@tmokmss @TikiTDO
feat(cli): support hotswapping Lambda functions that use Docker images (
9744c77 
aws#18319)

closes aws#18302

We must just update `ImageUri` with the new ECR image url.

PR for `InlineCode` hotswap: aws#18408
 
`----`

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
@tmokmss @TikiTDO
feat(cli): support hotswapping Lambda functions with inline code (aws…
edc4260 
…#18408)

Similarly to aws#18319, this PR supports hotswap of Lambda functions that use `InlineCode`.

`InlineCode` uses [CloudFormation `ZipFile` feature](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-lambda-function-code.html#:~:text=requires%3A%20No%20interruption-,ZipFile,-\(Node.js%20and). To emulate its behavior, we create a zip file of the provided inline code with its filename `index.js` or `index.py` according to the runtime (CFn only supports python or nodejs for ZipFile), and pass the file's binary buffer to Lambda API.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skinny85 skinny85 skinny85 approved these changes

@comcalvi comcalvi comcalvi approved these changes

Assignees

@skinny85 skinny85

@rix0rrr rix0rrr

@comcalvi comcalvi

Labels
package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tmokmss @aws-cdk-automation @skinny85 @comcalvi @rix0rrr