-
-
Notifications
You must be signed in to change notification settings - Fork 251
Add support for MongoDBContainer credentials #1070
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
Add support for MongoDBContainer credentials #1070
Conversation
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Also very interesting that 4.0 image fails tests with: I do not encounter this on local machine. Need to investigate more. |
7cab1ae to
31854b5
Compare
|
Ok, I decided to start from scratch here. I wrote a new |
Had to pin mongoose to I moved all logic in Credentials work with images from 4.0.1 up to 8.0 |
|
Tests with podman fail healthcheck even in older cases without credentials. I think my timeouts may be the reason? I have only docker installed locally. Will try later to test in podman too. |
|
Changed |
|
@digital88 do you mind resolving the conflicts? I would but I’m afk at the moment. Should just be a testcontainers version bump |
Resolved |
And resolved again 😄 |
|
Apologies @digital88, things are a bit busy at the moment, I know I need to review this PR, I will get to it soon. |
Let's not pin the mongoose version. It's pinned because of a Mongo version which was EOL Feb 2024, so when we need to update mongoose for some other reason we'll be stuck. As this is a new behaviour we don't need to test it with older Mongo versions as there are no existing users using it, so let's remove the credentials test for Mongo 4. The existing Mongo 4 test continues to pass so that's good enough from my perspective that this isn't a breaking change. If a user wants to use an older version of Mongoose so that it works with their Mongo 4 then great, but let's not tie ourselves to it. |
I unpinned mongoose. Made tests with creds to run just on mongo 8.0 |
|
Oh, sorry, TC patch version just went up again...Will fix it in a minute. |
|
Looks like I messed lockfile..I'll fix it. |
|
@digital88 Are there any more changes you'd like to make? |
I think it's good 👍 |
|
Thanks for the contribution, always a pleasure! |
Hopefully closes #988
This implementation supports credentials only when replica set is disabled. Because using credentials with replica set requires rather complicated setup I think it could be implemented later as separate PR. I'm not even sure why replica set was implemented in first place, as TC creates single container for running tests it does not make much sense IMO. Either way the old replica set code remains untouched and is used by default, so nothing should break for existing users.