feat(aws-lambda): AWS Lambda layer target#160
Merged
Conversation
Check and get environment variables `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY`, before publishing to AWS Lambda.
scefali
reviewed
Jan 7, 2021
scefali
reviewed
Jan 7, 2021
scefali
reviewed
Jan 8, 2021
If `AWS_LAYER_NAME` is set as an environment variable, it's used as the layer name. If not, the default layer name is used.
scefali
approved these changes
Jan 11, 2021
tonyo
suggested changes
Jan 11, 2021
tonyo
left a comment
Contributor
There was a problem hiding this comment.
Mostly minor comments.
Separate 👍 👍 for adding tests for the target!
Please also add the documentation for this new target to README.
- Delete `aws-lambda` target from `.craft.yml`. - Include start of line anchor (`^`) in the AWS Lambda regex. - Return promise instead of awaiting it inside the aws publish and add layer permissions methods. - Delete `TestArtifactProvider` and use `NoneArtifactProvider` instead.
rhcarvalho
reviewed
Jan 12, 2021
rhcarvalho
left a comment
Contributor
There was a problem hiding this comment.
Good stuff @iker-barriocanal
Dropped some comments to help move this to completion. Let's chat if I can help.
High level notes:
- Need to split clearly what is part of project config and what is part of the Craft implementation.
- Missing some documentation on how to use this new target in a craft-powered project (like sentry-javascript and sentry-python), essentially documenting what is the required config.
- Rename to `awsLambdaLayer`. - Extract hardcoded attributes to project settings in `.craft.yml` files: - Dist regex. - Compatible runtimes. - Layer name.
Extract layer permission variables to `.craft.yml` files.
- AWS SDK upgrade from v2 to v3. - AWS regions are now fetched using the EC2 client. - A license is now required in the project config. - Update tests to support SDKv3 and region fetch.
tonyo
reviewed
Jan 14, 2021
| // returned. Thus, both alternatives have been considered. | ||
| expect(publishedLayerVersion).toBe(undefined); | ||
| } catch (error) { | ||
| expect(error instanceof Error).toBe(true); |
Contributor
There was a problem hiding this comment.
To confirm: wouldn't this always be true?
If you want to confirm that a function throws an error, please consider using https://jestjs.io/docs/en/expect.html#tothrowerror
Contributor
Author
There was a problem hiding this comment.
The code inside catch, yes. publish will only throw if it's not running in dry-run mode; if it is, it doesn't throw anything and returns undefined. So, the approach is to run publish and expect to get undefined; if it throws an error before that, catch it. The method toThrow, afaik, requires to make the function call inside the expect, passing the test when the dry-run mode is off but failing when it's on.
rhcarvalho
approved these changes
Jan 14, 2021
rhcarvalho
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me, I think we can go ahead and update sentry-python and sentry-javascript to use this and make any adjustments later if need be. Good job @iker-barriocanal!
Comment on lines
+914
to
+921
| targets: | ||
| - name: aws-lambda-layer | ||
| includeNames: /^sentry-node-serverless-\d+(\.\d+)*\.zip$/ | ||
| layerName: SentryNodeServerlessSDK | ||
| compatibleRuntimes: | ||
| - nodejs10.x | ||
| - nodejs12.x | ||
| license: MIT |
Contributor
|
Re-requesting Anton if he still has any concerns from the points raised earlier. |
Contributor
|
@rhcarvalho Let's give Iker a break 😅 I've already left one a few minutes ago. |
Contributor
|
Oh sorry, I did not see it before I posted. |
tonyo
approved these changes
Jan 14, 2021
BYK
added a commit
that referenced
this pull request
May 22, 2026
…822) ## Summary Fixes Dependabot alert [#160](https://github.com/getsentry/craft/security/dependabot/160) (CVE-2026-3449, GHSA-vpq2-c234-7xj6). ## Problem `@tootallnate/once@2.0.0` is vulnerable to Incorrect Control Flow Scoping — Promises hang indefinitely when `AbortSignal` is used (control-flow leak). Severity: **Low** (CVSS 3.3). Dependency chain: ``` @google-cloud/storage@7.18.0 → teeny-request@9.0.0 → http-proxy-agent@5.0.0 → @tootallnate/once@2.0.0 ← vulnerable ``` Upgrading `@google-cloud/storage` alone doesn't fix this — the latest `7.19.0` still uses `teeny-request@^9.0.0` which pulls the same vulnerable chain. ## Fix Add a `pnpm.overrides` entry for `@tootallnate/once` to force `^2.0.1` (the patched version). This follows the same pattern already used for 8 other transitive dependency overrides in the project. The patched version satisfies the parent's declared range (`"2"` = `>=2.0.0 <3.0.0`), so there is no compatibility risk.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a target to publish a layer and sets its permissions in each region for the JS SDK.