Skip to content

Conversation

@dpilch
Copy link
Contributor

@dpilch dpilch commented Jul 31, 2024

Description of changes

With the CDK version upgrade the runtime version of the custom resource lambda changed from 16 to 18. With the runtime change the lambda code needed be changed to account for breaking changes between 16 and 18. However, the S3 key for the zip file containing the lambda code was not changed, even though the hash of code has changed. During the CFN deploy if the S3 key for the lambda code is unchanged the lambda code will not be updated.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-lambda-function-code.html

Changes to a deployment package in Amazon S3 or a container image in ECR are not detected automatically during stack updates. To update the function code, change the object key or version in the template.

This resulted in the runtime version being upgraded to 18, but the code was not updated (and stayed with the runtime version 16 compatible implementation). The lambda would then throw errors when executed.

This change updates the lambda code s3 key to append the runtime version. This will cause the next deploy to update the lambda code because the s3 key has changed.

Change in CLI repo: aws-amplify/amplify-cli#13872

CDK / CloudFormation Parameters Changed

Awaiter function code S3 key updated to include node runtime version.

Issue #, if available

#2730

Description of how you validated changes

Manual testing. This issue only presents itself when upgrading from an earlier version of the CLI with an existing awaiter function deployed. We do not have an E2E testing framework that allows multiple versions of the CLI to be used. Adding this testing would delay the release of this fix. The tests can be added at a later time.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Any CDK or CloudFormation parameter changes are called out explicitly

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

import { GitHubSourceActionInfo, PipelineWithAwaiter } from './pipeline-with-awaiter';

const PIPELINE_AWAITER_ZIP = 'custom-resource-pipeline-awaiter.zip';
const PIPELINE_AWAITER_ZIP = 'custom-resource-pipeline-awaiter-18.zip';
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be parameterized? Under what conditions would this number need to change in the future? What is 18 referring to?

Copy link
Contributor Author

@dpilch dpilch Jul 31, 2024

Choose a reason for hiding this comment

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

This name would need to change anytime this file is modified in the CLI repo. 18 is referring the node runtime version. The zip file was previously updated to be compatible with Node 18.

I think parametrization would be possible but I think would require larger changes to the API category and the CLI.

I chose the option to simply update the name (which will update the S3 key) because it would be the fastest method to release a fix for this while still being effective.

Copy link
Contributor

@phani-srikar phani-srikar Jul 31, 2024

Choose a reason for hiding this comment

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

This is the name of the key as stored in S3 by the CLI and this naming convention is shared b/w CLI and API plugins reference with the assumption on CLI side that the cloudformation provider would upload the lambda code in the deployment bucket using that key and we use it as part of the code pipeline state machine in our REST API deployments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Node 18 is EOL in 9 months. How do we prevent missing this 9 months from now? https://github.com/nodejs/Release

Copy link
Contributor

@phani-srikar phani-srikar Jul 31, 2024

Choose a reason for hiding this comment

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

This key doesn't require a change for every Lambda runtime update but when the underlying code is not compatible with older versions which coincidentally is the case from 16.x -> 18.x runtime since lambda changed it's default support of AWS SDK version from 2.x to 3.x as part of this runtime update reference. We should have an E2E test added to the migration tests that CLI has so that we can catch such cases where a runtime update requires code change. We could do this as a follow-up after the fix is verified to work for this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. I think an e2e validation after the fact sounds like a good plan.

@dpilch dpilch marked this pull request as ready for review August 1, 2024 15:37
@dpilch dpilch requested a review from a team as a code owner August 1, 2024 15:37
@dpilch dpilch merged commit 08db67b into main Aug 1, 2024
@dpilch dpilch deleted the update-awaiter-s3-key branch August 1, 2024 16:20
bobbyu99 pushed a commit that referenced this pull request Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants