-
Notifications
You must be signed in to change notification settings - Fork 23
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
Allow the extension layer to be applied separately from the runtime layers #131
Conversation
serverless/src/layer.ts
Outdated
export function applyExtensionLayer(region: string, lambdas: LambdaFunction[], extensionLayerVersion?: number) { | ||
const errors: string[] = []; | ||
lambdas.forEach((lambda) => { | ||
let lambdaExtensionLayerArn; | ||
|
||
if (extensionLayerVersion !== undefined) { |
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 feel like we should do here similarly to what we do above. If the extension version is not set, return an error. Otherwise, I think it will be real confusing for people.
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 have added this functionality and tested it.
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.
let's add an error if they dont set extension version. otherwise looks great!
We should also think about... does this change require a major version bump? I think the answer is yes.... but let's figure that out before we merge/release.
…bumping major version
extensionLayerVersion has always been optional as you don't need it if you are pushing your logs to cloudwatch and then using the cloudwatch forwarder. |
What does this PR do?
Currently there is no way to deploy with the cloudformation macro and only apply the extension.
This PR adds this ability when the user sets
addLayers
tofalse
the extension will still be applied.To override this feature a new configuration option has been made called 'addExtension
. This defaults to
true` ensuring the extension layer is applied.Motivation
Currently if a customer is providing their own lambda libraries (dd-trace, datadog-lambda-js) there is no way to exclude these layers from the deployment and still retain the extension layer. This adds that functionality for this edge case.
Testing Guidelines
In addition to the unit tests i used the release tool to upload my changes to a staging bucket that hosted the macro. Which i deployed to a sandbox aws environment and then created a new function using the macro. I had set
addLayers
tofalse
along with leavingaddExtension
to default. This produced a lambda with just the extension layer.I then modified my template.yaml file to set
addExtension
tofalse
this resulted in no lambda's being added.Additional Notes
I did not increment the version number in the template yaml file.
One thing to consider is that users that have
addLayers
tofalse
today and redeploy the macro will see a change in the expected behavior.Types of changes
Check all that apply