Add AWS+MySQL Rotator Agent #2
Conversation
This allows consumers of this lib to import without `--experimental-specifier-resolution=node`
Now that all relative imports have extensions, this isn't required.
| }); | ||
| } | ||
|
|
||
| export async function fetchS3KeySet(bucket = "doppler-keys") { |
There was a problem hiding this comment.
This bucket doesn't exist yet. We're in the process of getting a security review on the S3 keyset fetch approach and I'll post updates here
nmanoogian
force-pushed
the
nic/aws-mysql
branch
2 times, most recently
from
fb1e96c
to
f297131
Compare
| # Remove node_modules to be reinstalled later | ||
| RUN rm -r node_modules | ||
|
|
||
| FROM public.ecr.aws/lambda/nodejs:16 |
There was a problem hiding this comment.
Note that this Dockerfile is still meant to build a deployable image. We're currently considering switching to S3-based code bundle deployments. If we go that route, not much will change other than this file and some additional deploy scripts.
ENG-4295 Implement secret engine for rotation agent + AWS Postgres
Implement an engine to use in the generic rotation controller. |
utils/aws/lib/index.ts
Outdated
| export async function fetchS3KeySet(bucket = "doppler-keys") { | ||
| const s3Client = new S3({ forcePathStyle: true }); | ||
| const keyFile = await s3Client.getObject({ Bucket: bucket, Key: "secret-agents-jwks.json" }); |
There was a problem hiding this comment.
These can change later but I want to align on bucket/file naming. Ideally we follow the same pattern as our keys repo (i.e. $repo/secret-agents/jwks.json)
There was a problem hiding this comment.
I like that 👍 I'll change it preemptively
| @@ -0,0 +1,20 @@ | |||
| { | |||
| "name": "aws-utils", | |||
There was a problem hiding this comment.
Are we going to publish this? If not, probably best to add private: true
There was a problem hiding this comment.
Good point. We won't publish these immediately but in the future (6mo-1y), we'll probably be publishing these. Should we make them private: true for now and change them later? Or leave it as-is for now?
Regardless, we should definitely make sure that our licenses and other package metadata are all accurate
apps/aws-mysql-rotator/Dockerfile
Outdated
| COPY ./apps/aws-mysql-rotator/package*.json ./apps/aws-mysql-rotator/ | ||
| RUN npm install --workspace apps/aws-mysql-rotator | ||
|
|
||
| RUN ls -l node_modules |
There was a problem hiding this comment.
Yep, my bad -- missed this!
apps/aws-mysql-rotator/Dockerfile
Outdated
| COPY --from=build /usr/src/app . | ||
|
|
||
| # Only install production dependencies | ||
| RUN npm install --production |
There was a problem hiding this comment.
We probably want npm clean-install --production (I think it supports --production)
There was a problem hiding this comment.
Sounds good! Because we're doing this from a brand new image, I don't think this is strictly necessary but it's probably a good practice. I verified that clean-install does support the --production flag
apps/aws-mysql-rotator/app.ts
Outdated
| if (process.env.OVERRIDE_KEY_SET) { | ||
| keySetOption = { type: "local", keySet: JSON.parse(process.env.OVERRIDE_KEY_SET) }; | ||
| } else { | ||
| keySetOption = { type: "local", keySet: await fetchS3KeySet(process.env.OVERRIDE_KEY_SET_S3_BUCKET ?? undefined) }; |
There was a problem hiding this comment.
nit: is the ?? undefined needed?
There was a problem hiding this comment.
Nope, I was just trying to indicate that it was indeed optional. If you think that's clear already (given "override" in the name) then I'll drop it
| @@ -0,0 +1,134 @@ | |||
| import mysql, { QueryError } from "mysql2/promise"; | |||
There was a problem hiding this comment.
Is there a more "official" mysql package we can use?
There was a problem hiding this comment.
Good callout. I hunted around for mysql libs and found a few options and this is what I turned up:
- @mysql/xdevapi
⚠️ 1,182 weekly downloads⚠️ GPLv2 with FOSS license⚠️ Funky API
- mysql
- ✅ 864,748 weekly downloads
- ✅ MIT license
⚠️ Does not support MySQL 8 authentication
- mysql2
- ✅ 1,063,501 weekly downloads
- ✅ MIT license
- ✅ Supports MySQL 8 authentication and everything in the "Why MySQL2?" section
nmanoogian
force-pushed
the
nic/aws-mysql
branch
from
f297131
to
3b99c90
Compare
|
Force push addresses review feedback |
ENG-4294 Implement MySQL + AWS rotation agent v1
|
This PR adds an agent for rotating MySQL database credentials using an AWS Lambda.
Note that we're fetching our keyset from S3, see the security review doc for more details.
Closes ENG-4294