Support the Azure VM-assigned managed identity for automatic KMS credentials #1035
Conversation
…entials JAVA-4706
This unit test is not pulling enough weight given the success/failure integration tests we already have. Removing and simplifying the production code now that we no longer need to mock the endpoint.
Increase timeout
.evergreen/.evg.yml
Outdated
| export AZUREKMS_RESOURCEGROUP=${testazurekms_resourcegroup} | ||
| export AZUREKMS_VMNAME=${AZUREKMS_VMNAME} | ||
| export AZUREKMS_PRIVATEKEYPATH=/tmp/testazurekms_privatekey | ||
| AZUREKMS_CMD="MONGODB_URI='mongodb://localhost:27017' PROVIDER="azure" ./.evergreen/run-fle-on-demand-credential-test.sh" $DRIVERS_TOOLS/.evergreen/csfle/azurekms/run-command.sh |
There was a problem hiding this comment.
I wonder why did you miss KEY_NAME='${testazurekms_keyname}' KEY_VAULT_ENDPOINT='${testazurekms_keyvaultendpoint}'?
There was a problem hiding this comment.
Looks like those values are hardcoded in the integration test. Is that not ok?
There was a problem hiding this comment.
better to use env variables instead hardcoded values. That values are going to be changed in this ticket https://jira.mongodb.org/browse/BUILD-16154. Then, applying it in the driver will be easier with env variables instead of changing it in the code. NOTE: we will need to change the code in c# driver, my fault :(
DmitryLukyanov
left a comment
DmitryLukyanov
left a comment
There was a problem hiding this comment.
I don't see this part in this PR: mongodb/specifications@d6b8cce#diff-de01d398114f63480af23798e09ffca599aa758efd04f15a9beec89b4566041bR344. Has it been implemented somewhere else?
|
Agreed to defer credential caching to subsequent PR. Is there anything else missing besides that? |
|
@DmitryLukyanov ready for another look |
DmitryLukyanov
left a comment
DmitryLukyanov
left a comment
There was a problem hiding this comment.
LGTM (assuming tests passed)
2 participants
JAVA-4706
After discussing with Kevin, determined that given the coverage provided by the integration tests, there is not enough additional value in the unit tests (which require starting a mock server) to get them running in CI. So although they are implemented in the first commit, they are removed in the second one.