MONGOSH-523 - AutoEncryption only run on enterprise #545
Conversation
| !(buildInfo.modules && buildInfo.modules.indexOf('enterprise') !== -1) | ||
| ) { | ||
| await client.close(); | ||
| throw new MongoshRuntimeError('Cannot turn on automatic encryption for connections to non-enterprise hosts'); |
There was a problem hiding this comment.
I am not sure about the regex on gitVersion. At least for our Atlas cluster, this is the first part of the buildInfo output:
{
version: '4.2.11',
gitVersion: 'ea38428f0c6742c7c2c7f677e73d79e17a2aab96',
modules: [ 'enterprise' ],
...
}Why were you thinking of checking gitVersion in addition to modules?
Another minor thing: buildInfo.modules.indexOf('enterprise') !== -1 can be rewritten as buildInfo.modules.includes('enterprise'). I think Typescript also supports optional chaining so you can probably write the second condition as
buildInfo.modules?.includes('enterprise')As for the copy of the error message, I'd go for something like:
Automatic encryption is only available with Atlas and MongoDB Enterprise. I'll get that reviewed.
There was a problem hiding this comment.
Other minor thing: not sure if we do it for the other errors, and not sure if this is special, but would it make sense to put this in the i18n package and read it from there?
There was a problem hiding this comment.
I took it directly from the mongodb-build-info package, which is what we're using in the rest of the shell to determine if it's an enterprise cluster. I also found only the git hash without 'enterprise' when I tested locally, but I thought it could possibly be from older versions before they had modules in buildInfo. I have a very vague memory of Irina mentioning the git hash with enterprise, but I can't find that conversation. We may want to improve the check there as well (placeholder jira ticket).
Per the i18n error message, we don't yet use i18n for error messages everywhere but that's certainly a future feature we should track in the future.
There was a problem hiding this comment.
From this stack overflow it seems like it may happen sometimes that the gitversion has enterprise: https://stackoverflow.com/questions/33160352/how-to-check-if-mongodb-enterprise-is-in-use but it may be an older version than we support
There was a problem hiding this comment.
Ah, ok, it's cool then. Thank you for clarifying.
| * @param clientOptions {MongoClientOptions} | ||
| * @param mClient {MongoClient} | ||
| */ | ||
| export async function connectMongoClient(uri: string, clientOptions: MongoClientOptions, mClient = MongoClient): Promise<MongoClient | void> { |
There was a problem hiding this comment.
When would this not return a Promise<MongoClient>?
There was a problem hiding this comment.
Yes, it would! Good catch - I previously had it looking like
if (x) {
return y;
}
throw z
which I just learned you can't have the function signature return just the type of y. Then I refactored to avoid that issue, but then forgot to update the signature 🤦
The best solution I could find was connecting twice, once with the autoencryption options entirely removed. This connection runs buildInfo, and if it's not enterprise, then it will close the connection and error. If it is enterprise, then it will close the connection and reconnect with the autoencryption options included.
TODO: the message that is thrown should be written & reviewed by the legal team per @mmarcon