Add auth to mongo db container

[silaev]

Add Authentication to MongoDBContainer
Offical MongoDB doc reference

[silaev]

Addresses a request to use auth with MongoDBContainer discussed here

[eddumelendez]

@silaev thanks for the PR! and sorry for the delay. Can you update the branch and resolve the conflicts, please? There have been some changes since it was submitted 😬

[silaev]

@silaev thanks for the PR! and sorry for the delay. Can you update the branch and resolve the conflicts, please? There have been some changes since it was submitted 😬

Hi @eddumelendez, done

[eddumelendez]

Thanks for the PR! I have left some comments and suggestions. Let's update the getConnectionString and getReplicaSetUrl accordingly.

[eddumelendez]

Mongo already provides a convinient way to create a client.

MongoClients.create(
                        MongoClientSettings.builder()
                            .credential(MongoCredential.createCredential(usernameRestrictedAccess, "admin", passwordRestrictedAccess.toCharArray()))
                            .applyConnectionString(new ConnectionString(String.format("mongodb://%s:%d", mongoDBContainer.getHost(), mongoDBContainer.getFirstMappedPort()) ))
                            .build()
                    )

[silaev]

I'm not sure that it's a good idea here because:

• Exposing MongoClientSettings to the public API seems to be dangerous because a user might set a SRV DNS or other settings that MongoDBContainer does not support. I'd rather not allow a user to construct a URL on their side, it's safer to do delegate it to MongoDBContainer providing a URL string. Otherwise, we might receive some unexpected bug tickets here;

• ConnectionString is for the public API purpose to expose only databaseName, username and password for now. As apposed to MongoClientSettings, it doesn't require setting auth db source, database etc. which simplifies the public API and takes control of it

[eddumelendez]

I was not proposing to use Mongo API in MongoDBContainer. Just sharing how it can be done from the outside as in this suggestion

[eddumelendez]

I'd rather not allow a user to construct a URL on their side, it's safer to do delegate it to MongoDBContainer providing a URL string. Otherwise, we might receive some unexpected bug tickets here;

TBH, I do not expect users to do that but who knows.

I think our docs can be improved in order to make it clear

[silaev]

We provide a URL string for a user via clear API that we take control of. I reckon it's not a good idea to constantly update docs mentioning "it does not support srv etc."? In my opionin, it's better not to provide a way to set srv.

I was not proposing to use Mongo API in MongoDBContainer. Just sharing how it can be done from the outside as in this suggestion

I see your point, but it's not a part of our public API. This way a user can construct everything, but having an issue with it is on their hands

[silaev]

I'd rather not allow a user to construct a URL on their side, it's safer to do delegate it to MongoDBContainer providing a URL string. Otherwise, we might receive some unexpected bug tickets here;

TBH, I do not expect users to do that but who knows.

I think our docs can be improved in order to make it clear

I updated Java doc to make it clear

[kiview]

Sorry @silaev, but it is still unclear with me what is the issue with the approach suggested by @eddumelendez, since it is very close to how we do it for other modules.

So you think users will struggle doing it this way and construct invalid URLs? Especially since users would normally use getConnectionString(). Or maybe I am missing the point of the discussion?

I thought @eddumelendez's point was about not exposing MongoDBContainer.ConnectionString as a public API.

[silaev]

Hi @kiview The idea of this builder is to provide 3 params for now (might be more in the future): username, pass and db name for MongoDBContainer to generate a URL as simple as this:

MongoClients.create(
    mongoDBContainer.getReplicaSetUrl(
        MongoDBContainer.ConnectionString
            .builder()
            .username(usernameRestrictedAccess)
            .password(passwordRestrictedAccess)
            .build()
    )
)

It’s similar to withUsername(...), withPassword(…) and withDatabaseName(...) etc. builder methods in the PostgreSQLContainer to generate a URL via getJdbcUrl(). As I understand, generating URL inside a container is close to how we do it for other modules. There is already public String getReplicaSetUrl(final String databaseName) {}, so I followed the same approach with a param here public String getReplicaSetUrl(final ConnectionString connectionString).

@eddumelendez proposed a solution when a URL is generated outside of MongoDBContainer via MongoClientSettings which we do not control. Is it close to how we do it for other modules ? This way a user has to construct a URL on their own and know some additional data:

MongoClients.create(
    MongoClientSettings.builder()
        .credential(MongoCredential.createCredential(usernameRestrictedAccess, "admin", passwordRestrictedAccess.toCharArray()))
        .applyConnectionString(new ConnectionString(String.format("mongodb://%s:%d", mongoDBContainer.getHost(), mongoDBContainer.getFirstMappedPort()) ))
        .build()
)

The second variant seems to me as quite complex and error prone, so I'm more comfortable with the first one. Correct me if I'm wrong, but the whole idea of Testcontainers is to simplify testing experience which, as I see it, the first variant follows

[silaev]

Hi @kiview , @eddumelendez
I haven’t heard back from you after my last comment. Did I have a change to convince you to go with the simple builder approach?

[eddumelendez]

Hi, thanks for ping us, I was thinking about it last week. There is a difference with the builders in database modules due to those are used by JdbcDatabaseContainer for the specific wait strategy and, well, JDBC API is in the JDK itself.

With latest changes in the MongoDbContainer, I would suggest the following:

• Make authentication opt-in, withAuthenticationEnable(), instead of making it the default. Unless, a specific MongoDB version is forcing it, we can check on that version.
• Provide withUsername and withPassword as initially provided in the PR
• Do not provide a default key file. So, the user is aware of what's involved to run MongoDB with authentication enabled.
• Make it work with sharding, if it is possible
• Improve our javadocs and docs in general to show the correct usage of the module.

Testcontainers responsibility is to provide a ready to use service running in a container. There could be many configuration in the service itself which we do not master and we prefer a generic approach, meanwhile we are learning from uses cases, rather than do something at the beginning that can change later just because we are not aware of it.

I know the answer doesn't answer the last question but I think it is our best effort, meanwhile we also learn about MongoDB and how to use it. So, the builder can be revisited in the future. I'm also aware that this will involve more changes than initially expected but it is due to the evolution of the module itself.

[eddumelendez]

Suggested change

	mongoDBContainer.getReplicaSetUrl(
	MongoDBContainer.ConnectionString
	.builder()
	.username(usernameRestrictedAccess)
	.password(passwordRestrictedAccess)
	.build()
	MongoClientSettings.builder()
	.credential(MongoCredential.createCredential(usernameRestrictedAccess, "admin", passwordRestrictedAccess.toCharArray()))
	.applyConnectionString(new ConnectionString(String.format("mongodb://%s:%d", mongoDBContainer.getHost(), mongoDBContainer.getFirstMappedPort()) ))
	.build()

[silaev]

pls, see the above mentioned discussion about MongoClientSettings

[silaev]

@eddumelendez, thanks for the review. I address all the discussions. Please, let me know if anything else is needed from me