Skip to content

CDRIVER-4454 add integration tests for Azure KMS #1124

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

Merged
merged 32 commits into from
Nov 3, 2022

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Oct 19, 2022

Summary

Here is an Evergreen patch build with the new tasks: https://spruce.mongodb.com/version/63500fcbd1fe071dfc2682da/

Notes

The additions to the Python evergreen generation scripts are minimal. IMO the Python scripts are not significantly more readable than the Evergreen config YML. For this PR, I found it easier to start with Evergreen YML, then translate to the Python scripts.

@kevinAlbs kevinAlbs changed the title DRIVERS-2411 CDRIVER-4454 add integration tests for Azure KMS Oct 19, 2022
set -o errexit
# TODO: update once https://github.com/mongodb-labs/drivers-evergreen-tools/pull/239 is merged.
git clone https://github.com/kevinAlbs/drivers-evergreen-tools.git --branch DRIVERS-2411 drivers-evergreen-tools
DRIVER_TOOLS=$(pwd)/drivers-evergreen-tools
Copy link

@DmitryLukyanov DmitryLukyanov Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRIVERS_TOOLS?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. DRIVERS_TOOLS is consistent with other naming in the config.

export AZUREKMS_RESOURCEGROUP=${testazurekms_resourcegroup}
export AZUREKMS_VMNAME=${AZUREKMS_VMNAME}
export AZUREKMS_PRIVATEKEYPATH=/tmp/testazurekms_privatekey
DRIVER_TOOLS=$(pwd)/drivers-evergreen-tools

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: If I understand this code correctly, this looks unnecessary if you use DRIVERS_TOOLS env variable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRIVERS_TOOLS is not defined before in this task.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it

export AZUREKMS_RESOURCEGROUP=${testazurekms_resourcegroup}
export AZUREKMS_VMNAME=${AZUREKMS_VMNAME}
export AZUREKMS_PRIVATEKEYPATH=/tmp/testazurekms_privatekey
DRIVERS_TOOLS=$(pwd)/drivers-evergreen-tools

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: if you use DRIVERS_TOOLS, this value should already be assigned, at least this is what I saw in the c# driver (same in other places)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed your explanation above. It's a bit strange, but ok :)

@kevinAlbs kevinAlbs marked this pull request as ready for review October 21, 2022 20:42
Copy link
Contributor

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delayed review. Have some minor change requests.

@kevinAlbs kevinAlbs merged commit 4a6de77 into mongodb:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants