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

Use secrets manager to read serverless credentials #1237

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Feb 27, 2024

This fixes the serverless build after the credentials were rotated.

Note: the change to use subprocess.exec instead of shell.exec is not related to the original issue, but it's recommended by the evergreen team.

@alcaeus alcaeus requested a review from a team as a code owner February 27, 2024 19:58
@alcaeus alcaeus requested a review from jmikola February 27, 2024 19:58
bash ${DRIVERS_TOOLS}/.evergreen/serverless/create-instance.sh
binary: bash
args:
- ${DRIVERS_TOOLS}/.evergreen/serverless/create-instance.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

setup-secrets is now automatically called in create-instance

export PATH="${PHP_PATH}/bin:$PATH"

. ${DRIVERS_TOOLS}/.evergreen/serverless/secrets-export.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

This file is created by setup-secrets and contains the secrets needed to run tests

Comment on lines +282 to +283
export MONGODB_USERNAME=$SERVERLESS_ATLAS_USER
export MONGODB_PASSWORD=$SERVERLESS_ATLAS_PASSWORD
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, it took me too long to remember that $FOO refers to an environment variable named FOO, whereas ${FOO} refers to an evergreen expansion named FOO 🤦‍♂️

@@ -274,12 +274,15 @@ functions:
export KMS_TLS_CA_FILE="${client_side_encryption_kms_tls_ca_file}"
export KMS_TLS_CERTIFICATE_KEY_FILE="${client_side_encryption_kms_tls_certificate_key_file}"
export MONGODB_IS_SERVERLESS=on
export MONGODB_USERNAME=${SERVERLESS_ATLAS_USER}
export MONGODB_PASSWORD=${SERVERLESS_ATLAS_PASSWORD}
export MONGODB_URI="${SERVERLESS_URI}"
Copy link
Member

Choose a reason for hiding this comment

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

Why was this moved to an export instead of a direct env var for calling run-tests.sh?

Copy link
Member Author

Choose a reason for hiding this comment

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

No particular reason TBH, I was moving some stuff around in the process which may have stuck.

@jmikola
Copy link
Member

jmikola commented Feb 28, 2024

Just reconfigured the patch build to include both serverless tasks: https://spruce.mongodb.com/version/65de3efcc9ec44736b863734/tasks

@alcaeus alcaeus force-pushed the fix-serverless-credentials branch from 8d7c739 to b0b64e4 Compare February 29, 2024 08:48
@alcaeus
Copy link
Member Author

alcaeus commented Feb 29, 2024

Rebased on v1.17 and re-scheduled the two serverless tasks in the PR patch.

@alcaeus alcaeus requested a review from jmikola February 29, 2024 08:50
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

My question in #1237 (comment) is outstanding but I won't argue with the build status.

@alcaeus
Copy link
Member Author

alcaeus commented Feb 29, 2024

evergreen keep-definitions

@alcaeus alcaeus merged commit 5b3a698 into mongodb:v1.17 Mar 1, 2024
23 checks passed
@alcaeus alcaeus deleted the fix-serverless-credentials branch March 1, 2024 09:27
alcaeus added a commit that referenced this pull request Mar 1, 2024
* v1.17:
  Use secrets manager to read serverless credentials (#1237)
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.

2 participants