-
Notifications
You must be signed in to change notification settings - Fork 918
GODRIVER-3548 Test MONGODB-X509 on cloud-dev #2166
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
GODRIVER-3548 Test MONGODB-X509 on cloud-dev #2166
Conversation
API Change ReportNo changes found! |
🧪 Performance ResultsCommit SHA: 9967f6eThe following benchmark tests for version 689cf1590e814800078fb03d had statistically significant changes (i.e., |z-score| > 1.96):
For a comprehensive view of all microbenchmark results for this PR's commit, please check out the Evergreen perf task for this patch. |
de1513c
to
33324e1
Compare
654608b
to
c687f04
Compare
4db6f53
to
9922f9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests currently fail with
Environment variable ATLAS_X509_DEV_CERT_BASE64 is not set
and
Environment variable ATLAS_X509_DEV_CERT_NOUSER_BASE64 is not set
The rest of the tests are skipped because the env vars are not set. It's not clear where the env vars are expected to be set.
test-oidc-remote: bash etc/run-oidc-remote-test.sh | ||
|
||
test-atlas-connect: | ||
- go test -v -run ^TestAtlas$ go.mongodb.org/mongo-driver/v2/internal/cmd/testatlas -args "$ATLAS_REPL" "$ATLAS_SHRD" "$ATLAS_FREE" "$ATLAS_TLS11" "$ATLAS_TLS12" "$ATLAS_SERVERLESS" "$ATLAS_SRV_REPL" "$ATLAS_SRV_SHRD" "$ATLAS_SRV_FREE" "$ATLAS_SRV_TLS11" "$ATLAS_SRV_TLS12" "$ATLAS_SRV_SERVERLESS" >> test.suite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems none of the URI env vars were previously populated, effectively making this test a no-op. It's unclear when that change happened because the test passes silently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests do pass and are not skipped on the correct task: https://parsley.mongodb.com/test/mongo_go_driver_atlas_test_atlas_test_patch_5b79d946e0414b30e7a4ce53beedd43abef23eb2_689a9b41febc2d0007ffba2d_25_08_12_01_39_16/0/f9ccdfa9f8beaef7f7d584890b0b7de3?bookmarks=0,31&shareLine=1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see that now, thanks for the clarification!
internal/cmd/testatlas/atlas_test.go
Outdated
if uri == "" { | ||
t.Skipf("Environment variable %q is not set", tc.envVar) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipping if the env vars aren't set will cause the tests to silently pass if the env vars somehow get unset, even if there is a bug. Is there a way to not skip in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add a build tag and remove the skip option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a great solution!
The variables can be set using the secrets manager, here's how we do this for PHPC: https://github.com/mongodb/mongo-php-driver/blob/b6ed52c2b83be428d8eb093f402c49ccf96c57cf/.evergreen/config/test-tasks.yml#L5..L16 We then include the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds test support for MONGODB-X509 authentication on cloud-dev environments to ensure OpenSSL 3 compatibility. The changes restructure the Atlas test suite to use table-driven tests and adds specific X509 authentication test cases.
- Refactors the Atlas test suite from command-line argument based testing to table-driven tests
- Adds new test cases for X509 authentication including positive and negative scenarios
- Implements helper functions for creating temporary certificate files from base64-encoded environment variables
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
internal/require/require.go | Adds NotEmpty assertion function for test requirements |
internal/assert/assertions.go | Adds NotEmpty assertion function for test assertions |
internal/cmd/testatlas/atlas_test.go | Restructures Atlas tests to table-driven format and adds X509 test cases |
Taskfile.yml | Updates test command to use build tags instead of command-line arguments |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
GODRIVER-3548
Summary
Test MONGODB-X509 auth on cloud-dev to ensure auth will succeed using OpenSSL 3.
Background & Motivation
NA