-
Notifications
You must be signed in to change notification settings - Fork 1
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that 👍 I'll change it preemptively
@@ -0,0 +1,20 @@ | |||
{ | |||
"name": "aws-utils", |
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.
Are we going to publish this? If not, probably best to add private: true
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For debugging?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want npm clean-install --production
(I think it supports --production
)
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is the ?? undefined
needed?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a more "official" mysql package we can use?
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.
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
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