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.

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

feat(NODE-5470)!: convert remaining FLE to TS and drop support for onKMSProvidersRefresh #3787

Merged

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Jul 27, 2023

Description

What is changing?

The remaining FLE files and tests are converted to TS. Additional changes:

  • buffer_pool.js was removed and now FLE code imports the buffer pool from utils.ts.
  • once and child_process are lazily imported in the state machine and mongodbcyptd manager, respectively
  • lint has been re-enabled and once again covers all tests and src
  • databaseNamespace and collectionNamespace have been removed in favor of MongoDBNamespace.

One other notable change is that when we fail to get kms provider information, the error thrown used to attach an originalError property. I adjusted this to use the error's cause, since Node16+ and MongoCryptErrors support the cause property.

I also removed support for onKMSProvidersRefresh here.

Is there new documentation needed for these changes?

No, documentation is coming in a follow up.

What is the motivation for this change?

Release Highlight

ClientEncryption.onKMSProvidersRefresh function removed

ClientEncryption.onKMSProvidersRefresh was added as a public API in version 2.3.0 of mongodb-client-encryption to allow for automatic refresh of KMS provider credentials. Subsequently, we've added the capability to automatically refresh KMS credentials using the KMS provider's preferred refresh mechanism, and onKMSProviderRefresh is no longer used.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/clientEncryption.ts Outdated Show resolved Hide resolved
src/client-side-encryption/common.js Show resolved Hide resolved
src/client-side-encryption/stateMachine.ts Outdated Show resolved Hide resolved
src/client-side-encryption/stateMachine.ts Outdated Show resolved Hide resolved
src/client-side-encryption/stateMachine.ts Outdated Show resolved Hide resolved
@baileympearson baileympearson marked this pull request as ready for review July 31, 2023 15:31
@baileympearson baileympearson changed the title Node 5470 typescript remaining fle feat!(NODE-5470): convert remaining FLE to TS and drop support for onKMSProvidersRefresh Jul 31, 2023
@nbbeeken nbbeeken self-requested a review July 31, 2023 16:35
@nbbeeken nbbeeken self-assigned this Jul 31, 2023
@nbbeeken nbbeeken added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jul 31, 2023
@nbbeeken nbbeeken changed the title feat!(NODE-5470): convert remaining FLE to TS and drop support for onKMSProvidersRefresh feat(NODE-5470)!: convert remaining FLE to TS and drop support for onKMSProvidersRefresh Jul 31, 2023
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/stateMachine.ts Outdated Show resolved Hide resolved
src/client-side-encryption/stateMachine.ts Outdated Show resolved Hide resolved
src/client-side-encryption/stateMachine.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/autoEncrypter.ts Outdated Show resolved Hide resolved
src/client-side-encryption/mongocryptdManager.ts Outdated Show resolved Hide resolved
src/client-side-encryption/mongocryptdManager.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
@nbbeeken nbbeeken merged commit 844aa52 into mongodb:main Aug 3, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants