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

adds layers to function #1835

Open

ykethan wants to merge 17 commits into aws-amplify:main from ykethan:reference-layer

+301 −18

Member

ykethan commented Aug 8, 2024 •

edited

Loading

Problem

Issue number, if available:
RFC: #1549

Changes

adds ability to reference Lambda layers and attach to a function.
Can attach up to 5 layers on a function. Auto sets the layer keys as external modules on the function.

example:

resource.ts

import { defineFunction } from "@aws-amplify/backend";

export const sayHello = defineFunction({
  name: "say-hello1",
  entry: "./handler.ts",
  layers: {
   "@aws-lambda-powertools/logger":
      "arn:aws:lambda:us-east-1:094274105915:layer:AWSLambdaPowertoolsTypeScriptV2:11",
  },
});

handler.ts. keep using your dependancy without having to use /opt as the NODE_PATH would point to /opt: https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html#configuration-envvars-runtime

import { Logger } from "@aws-lambda-powertools/logger";

const logger = new Logger({ serviceName: "serverlessAirline" });

export const handler = async (): Promise<void> => {
  logger.info("Hello World");
};

Validation

Checklist

If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
If this PR requires a change to the Project Architecture README, I have included that update in this PR.
If this PR requires a docs update, I have linked to that docs PR above.
If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

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


          adds referenceLayer to function

eebe2b2

ykethan requested a review from a team as a code owner

August 8, 2024 14:56

changeset-bot bot commented Aug 8, 2024 •

edited

Loading

🦋 Changeset detected

Latest commit: 18d5bbb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@aws-amplify/backend-function	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

sobolk reviewed

View reviewed changes

packages/backend-function/src/factory.ts Outdated

Comment on lines 127 to 139

+                /**
+                 * Attach Lambda layers to a function
+                 * @example
+                 * import { referenceFunctionLayer } from "@aws-amplify/backend";
+                 *
+                 * layers: {
+                 *    myModule: referenceFunctionLayer("arn:aws:lambda:<current-region>:<account-id>:layer:myLayer:1")
+                 * },
+                 *
+                 * The object is keyed by the module name hosted on your existing layer and a value that references to an existing layer using an ARN. The keys will be externalized and available via your layer at runtime
+                 * Maximum of 5 layers can be attached to a function. Layers must be in the same region as the function.
+                 */
+                layers?: FunctionLayerReferences;

Member

sobolk Aug 8, 2024

Do we need referenceFunctionLayer ?

Can't we just

layers: {
   myModule: "arn:aws:lambda:<current-region>:<account-id>:layer:myLayer:1"
},

and have construct/factory validate and resolve necessary pieces inside ?

Member Author

ykethan Aug 8, 2024 •

edited

Loading

this was built according to the RFC requirements
#1549

ykethan added 3 commits

August 8, 2024 11:11


          api.md and lint

d67f787


          api.md

e5a1252


          remove referenceLayer

5a8621a

ykethan requested a review from a team as a code owner

August 8, 2024 23:27

ykethan added 2 commits

August 8, 2024 19:33


          change set

2520aeb


          update error to Amplify error type

539083f

ykethan requested a review from sobolk

August 9, 2024 21:45

josefaidt reviewed

View reviewed changes

packages/backend-function/src/factory.ts Outdated Show resolved Hide resolved

josefaidt reviewed

View reviewed changes

packages/backend-function/src/factory.ts Show resolved Hide resolved

josefaidt reviewed

View reviewed changes

packages/backend-function/src/factory.ts Outdated Show resolved Hide resolved

ykethan added 2 commits

August 11, 2024 14:14


          update jsdocs

aa26f26


          Merge branch 'main' of github.com:ykethan/amplify-backend into refere…

da6bdf1

…nce-layer

josefaidt reviewed

View reviewed changes

packages/backend-function/src/factory.ts Outdated Show resolved Hide resolved

josefaidt reviewed

View reviewed changes

packages/backend-function/src/factory.ts Outdated Show resolved Hide resolved

ykethan added 3 commits

August 14, 2024 09:15


          Merge branch 'main' of github.com:ykethan/amplify-backend into refere…

33b4b64

…nce-layer


          update jsdoc

4c20954


          change jsdoc

ykethan changed the title ~~adds referenceLayer to function~~ adds layers to function

Amplifiyer requested changes

View reviewed changes

packages/backend-function/src/factory.ts Outdated

+                /**
+                 * Attach Lambda layers to a function
+                 * - The object is keyed by the module name hosted on your existing layer and a value that references to an existing layer using an ARN. The keys will be externalized and available via your layer at runtime

Contributor

Amplifiyer Aug 23, 2024

What does this "module name hosted on your existing layer" mean? Do customers need to do something?

Member Author

ykethan Aug 26, 2024

updated the jsdoc to be more verbose

packages/backend-function/src/factory.ts Outdated

Comment on lines 284 to 286

+              type HydratedFunctionProps = Required<FunctionProps> & {
+                layers: FunctionLayerReferences;
+              };

Contributor

Amplifiyer Aug 23, 2024

why do we need this? Wouldn't layers be automatically be present and required in HydratedFunctionProps

Member Author

ykethan Aug 26, 2024

removed this

packages/backend-function/src/factory.ts Outdated

Comment on lines 300 to 304

+                  const resolvedLayers = resolveLayers(
+                    this.props.layers,
+                    scope,
+                    this.props.name
+                  );

Contributor

Amplifiyer Aug 23, 2024

should this be part of hydrateDefaults?

Member Author

ykethan Aug 26, 2024

the layers requires the scope for the fromLayerVersionArn to convert to a ILayer.

packages/backend-function/src/reference_layer.ts Outdated

Comment on lines 9 to 18

+              export const arnPattern = new RegExp(
+                '^(arn:(aws[a-zA-Z-]*)?:lambda:[a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\\d{1}:\\d{12}:layer:[a-zA-Z0-9-_]+:[0-9]+)|(arn:[a-zA-Z0-9-]+:lambda:::awslayer:[a-zA-Z0-9-_]+)$'
+              );
+              /**
+               * Checks if the provided ARN is valid.
+               */
+              export const isValidLayerArn = (arn: string): boolean => {
+                return arnPattern.test(arn);
+              };

Contributor

Amplifiyer Aug 23, 2024

do we need these methods exported or used outside of this file? If not, can we bring them into the class as privates.

Member Author

ykethan Aug 26, 2024

separated the logic and added them as private.

packages/backend-function/src/reference_layer.ts Outdated

Comment on lines 9 to 11

+              export const arnPattern = new RegExp(
+                '^(arn:(aws[a-zA-Z-]*)?:lambda:[a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\\d{1}:\\d{12}:layer:[a-zA-Z0-9-_]+:[0-9]+)|(arn:[a-zA-Z0-9-]+:lambda:::awslayer:[a-zA-Z0-9-_]+)$'
+              );

Contributor

Amplifiyer Aug 23, 2024

Can you add the source of this ARN in the documentation.

Member Author

ykethan Aug 26, 2024

got this regex from CFN error, but modified this now to be similar to the docs now and linked the documentation.

packages/backend-function/src/reference_layer.ts Outdated

Comment on lines 70 to 72

+                return new Set<FunctionLayerArn>(
+                  Array.from(uniqueArns).map(parseFunctionLayerArn)
+                );

Contributor

Amplifiyer Aug 23, 2024

If the method is called validate it shouldn't do anything else but validate what it's supposed to do. This parsing/mapping shouldn't be part of it.

packages/backend-function/src/reference_layer.ts Outdated

+              /**
+               * Class to represent and handle Lambda Layer ARNs.
+               */
+              export class FunctionLayerArn {

Contributor

Amplifiyer Aug 23, 2024

Why do we need a class to represent the arn? It seems like it's just doing validation.

Recommendation: Create a class FunctionLayerParser/FunctionLayerReferenceAdapter (or something like that) with one public method parse/adapt that takes in the user input, i.e. Record<String, String> and does several validations and finally produces ILayerVersion[]

Member Author

ykethan Aug 26, 2024

recreated this as a FunctionLayerParser class with parse as suggested

sobolk reviewed

View reviewed changes

packages/backend-function/API.md Outdated

Comment on lines 24 to 25

		// @public (undocumented)
		export type FunctionLayerReferences = Record<string, string>;

Member

sobolk Aug 10, 2024

If we're not inventing anything to constrain this to ARN we should inline this type and rely on jsdocs.

Member Author

ykethan Aug 26, 2024

inlined this. But to note there is a way we can add a template string literal this but would require a workaround wasnt comfortable using this as unsure if it will be removed: microsoft/TypeScript#29729 (comment)

https://www.typescriptlang.org/play/?#code/C4TwDgpgBAYghgGwQIzgYwNYGVgCcCWAdgOZQC8UAznkaQGRQDeUcuhAMvhhAPwBcLQiCgBfANwBYAFChIsAK6E0wfAHsOcEBFwBBNuSgADVoT5wA7pT4I4AW2QATOHwAkjagRIjX7ml+ua2j4etN5uIV6GUAA+sIgo6Nh+xJIy4NAwispqGlq6bADyBMREiAbGbGaWAfZOwclhvp7E3jZ59c2NES2GqdJo6tRQwBDUAplKKursgfmERfglhGUUAOQmVVY2tc4jY3vAAIwB7QcATHwHAMyrqQOEQ+fjWVO52nqEBuuVFlt2jrtRsBLkDjm0gk9rrdpGBcKo0KNKAA6CCEABuSIOAFF0QYDndBsBhkCrs9JjkZnkPgslisoLD4YiUejMUCcWiWJQqMkCQ8iQcACxk7LTWYfAwMhGUZGojHY3FwLndPpSaTSIA

.changeset/calm-buttons-sort.md Outdated

+              '@aws-amplify/backend': minor
+              ---
+              adds referenceLayer to function

Member

sobolk Aug 10, 2024

this and some code comments don't seem to be accurate now.

ykethan added 3 commits

August 24, 2024 22:14


          Merge branch 'main' of github.com:ykethan/amplify-backend into refere…

5c3ac20

…nce-layer


          update layers

ad8eac3


          update layers

11d92f7

ykethan requested review from Amplifiyer, josefaidt and sobolk

August 26, 2024 17:27

Amplifiyer reviewed

View reviewed changes

.changeset/serious-dragons-attack.md Outdated

+              '@aws-amplify/backend-function': minor
+              ---
+              adds layers to defineFunction

Contributor

Amplifiyer Aug 28, 2024

Suggested change

      
            adds layers to defineFunction
          
            adds support to reference existing layers in defineFunction

packages/backend-function/src/layer_parser.ts Outdated

Comment on lines 25 to 36

+                  const uniqueArns = new Set<string>(Object.values(layers));
+                  this.validateLayerCount(uniqueArns);
+                  for (const [key, arn] of Object.entries(layers)) {
+                    if (!this.isValidLayerArn(arn)) {
+                      throw new AmplifyUserError('InvalidLayerArnFormatError', {
+                        message: `Invalid ARN format for layer: ${arn}`,
+                        resolution: `Update the layer ARN with the expected format: arn:aws:lambda:<current-region>:<account-id>:layer:<layer-name>:<version> for function: ${functionName}`,
+                      });
+                    }
+                    validLayers[key] = arn;
+                  }

Contributor

Amplifiyer Aug 28, 2024

We are validating on the count of uniqueArns not the number of entries in the map. But later we are not deduping it later on. So for example if I provide a map of

{
  key1: arn1,
  key2: arn1,
  key3: arn1,
  key4: arn1,
  key5: arn1,
  key6: arn1,
}

our validation will succeed, but we will be passing/creating 6 reference layers to the NodeJSFunction constructor which may fail later.

Ideally we should dedupe here on the ARN and provide any of the key here since the key is only used for generating the name of the layer.

Member Author

ykethan Aug 28, 2024

oops thanks for the catch, updated this and also added a test to ensure unique Arn's are passed.

ykethan added 2 commits

August 28, 2024 13:06


          update to check for unqiue arns and adds a test

286757f


          Merge branch 'main' of github.com:ykethan/amplify-backend into refere…

5e60dc7

…nce-layer

ykethan requested a review from Amplifiyer

August 28, 2024 17:28

Amplifiyer reviewed

View reviewed changes

packages/backend-function/src/layer_parser.ts Outdated

Comment on lines 24 to 37

+                  const uniqueArns = new Map<string, string>();
+                  for (const [key, arn] of Object.entries(layers)) {
+                    if (!this.isValidLayerArn(arn)) {
+                      throw new AmplifyUserError('InvalidLayerArnFormatError', {
+                        message: `Invalid ARN format for layer: ${arn}`,
+                        resolution: `Update the layer ARN with the expected format: arn:aws:lambda:<current-region>:<account-id>:layer:<layer-name>:<version> for function: ${functionName}`,
+                      });
+                    }
+                    // Add ARN to the Map with the first encountered key
+                    if (!uniqueArns.has(arn)) {
+                      uniqueArns.set(arn, key);
+                    }
+                  }

Contributor

Amplifiyer Aug 28, 2024

You can simplify this a bit by using just a Set instead of Map and build your validLayers directly in this method.


          update to use set

18d5bbb

ykethan requested a review from Amplifiyer

August 28, 2024 18:49

Amplifiyer approved these changes

View reviewed changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet