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

Use the new probot package @probot/get-private-key #52

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Use the new probot package @probot/get-private-key #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,22 @@ module.exports.probot = serverless(appFn);

This package moves the functionality of `probot run` into a handler suitable for usage on AWS Lambda + API Gateway. Follow the documentation on [Environment Configuration](https://probot.github.io/docs/configuration/) to setup your app's environment variables. You can add these to `.env`, but for security reasons you may want to use the [AWS CLI](https://aws.amazon.com/cli/) or [Serverless Framework](https://github.com/serverless/serverless) to set Environment Variables for the function so you don't have to include any secrets in the deployed package.

To use `.env` files with the [Serverless Framework](https://github.com/serverless/serverless), you can install the [serverless-dotenv-plugin](https://www.serverless.com/plugins/serverless-dotenv-plugin). This will take care of keeping your secrets out of your deployed package.

### Serverless dotenv plugin usage
```yaml
plugins:
- serverless-dotenv-plugin # Load .env as environment variables

provider:
name: aws
runtime: nodejs12.x
```

For the private key, since AWS environment variables cannot be multiline strings, you could [Base64 encode](https://nodejs.org/api/buffer.html#buffer_buffers_and_character_encodings) the `.pem` file you get from the GitHub App or use [KMS](https://aws.amazon.com/kms/) to encrypt and store the key.



## Differences from `probot run`

#### Local Development
Expand Down
4 changes: 2 additions & 2 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { Probot } = require("probot");
const { resolve } = require("probot/lib/helpers/resolve-app-function");
const { findPrivateKey } = require("probot/lib/helpers/get-private-key");
const { getPrivateKey } = require("@probot/get-private-key");
const { template } = require("./views/probot");

let probot;
Expand All @@ -11,7 +11,7 @@ const loadProbot = (appFn) => {
new Probot({
id: process.env.APP_ID,
secret: process.env.WEBHOOK_SECRET,
privateKey: findPrivateKey(),
privateKey: getPrivateKey(),
Copy link
Contributor

Choose a reason for hiding this comment

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

how would you usually configure the private key on lambda? I assume you wouldn't upload a *.pem file, so the only option really is to define the PRIVATE_KEY option? I think I'd remove the getPrivateKey method altogether and instead set

Suggested change
privateKey: getPrivateKey(),
privateKey: process.env.PRIVATE_KEY,

Copy link
Contributor Author

@mattkinnersley mattkinnersley Nov 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Given that getPrivateKey() does return process.env.PRIVATE_KEY as one of it's options and in the README there is a bit about using a .pem, I figured it would be safer to use this new package. But you're right, whenever I use lambda with serverless, I use the serverless-dotenv-plugin with .env files (also added this to the README).

I can't say for a fact it is impossible to use a *.pem so I thought this would give a bit of flexibility. As things stand, this package is unusable with the latest @probot which is the reason for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

As things stand, this package is unusable with the latest @probot which is the reason for this PR.

I agree, let's merge this PR and make a follow up PR for further discussion

});

if (typeof appFn === "string") {
Expand Down