-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
ci(NODE-6728): test FLE on Alpine linux #4417
base: main
Are you sure you want to change the base?
Conversation
e1b4a9e
to
dae0573
Compare
@@ -94,7 +94,6 @@ | |||
"express": "^4.21.2", | |||
"gcp-metadata": "^5.3.0", | |||
"js-yaml": "^4.1.0", | |||
"kerberos": "^2.2.1", | |||
"mocha": "^10.8.2", |
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.
Kerberos fails to install on alpine because we don't support it.
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 was explicitly added in #4411 . I guess it wasn't 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.
It looks like I forgot to update the lockfile.
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.
Sorry for the force push - I had to rebase with main. But fixed.
if (process.env.MONGODB_API_VERSION) { | ||
options.serverApi = process.env.MONGODB_API_VERSION as MongoClientOptions['serverApi']; | ||
} | ||
function makeClient( |
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.
The capturing mongoclient causes issues because clients are not instantiated with configuration.newClient().
const AWS_REGION = process.env.AWS_REGION; | ||
const AWS_CMK_ID = process.env.AWS_CMK_ID; | ||
const AWS_REGION = 'us-east-1'; | ||
const AWS_CMK_ID = 'dummy-cmk-id'; |
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 values come from our evergreen project, not secrets manager. These tests don't actually need valid secrets - these dummy values suffice.
@@ -186,7 +188,7 @@ const testConfigBeforeHook = async function () { | |||
adl: this.configuration.buildInfo.dataLake | |||
? this.configuration.buildInfo.dataLake.version | |||
: false, | |||
kerberos: process.env.KRB5_PRINCIPAL != null, | |||
kerberos: process.env.PRINCIPAL != null, |
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 was no longer accurate after migrating kerberos to secrets manager.
144f2d2
to
4edeb45
Compare
|
||
const kmsProviders = { | ||
aws: { accessKeyId: AWS_ACCESS_KEY_ID, secretAccessKey: AWS_SECRET_ACCESS_KEY } | ||
local: { key: Buffer.alloc(96) } |
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 tests still rely on expansions because these were node-specific unit tests. This fails without additional environment setup in the docker container because the secrets aren't available in secrets-export.sh.
I chose to rewrite these tests to be environment-independent because they're testing keyAltNames functionality, not AWS KMS integration.
44cf8fd
to
1f1fa50
Compare
Description
What is changing?
A new variant that runs FLE tests in a docker container on Alpine linux has been added.
Is there new documentation needed for these changes?
What is the motivation for this change?
Release Highlight
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript