testcontainers / testcontainers-dotnet

A library to support tests with throwaway instances of Docker containers for all compatible .NET Standard versions.
https://dotnet.testcontainers.org
MIT License
3.77k stars 271 forks source link

[Enhancement]: Add method to return CosmosDb connection string for internal network #1060

Closed goldsam closed 9 months ago

goldsam commented 10 months ago

Problem

Right now, CosmosDbContainer has a string GetConnectionString() method which returns a connection string for use on the host. It would be useful if CosmosDbContainer also had a method returning a connection string usable within a network the container is attached to. Ideally, this method would work before the container is started so that it can be used to build other containers which rely on this connection string.

Solution

I propose adding a method to class CosmosDbContainer which is functionally equivalent to:

private string GetInternalConnectionString(string networkAlias)
{
    var properties = new Dictionary<string, string>
    {
        ["AccountEndpoint"] = new UriBuilder(Uri.UriSchemeHttps, networkAlias, CosmosDbBuilder.CosmosDbPort).ToString(),
        ["AccountKey"] = CosmosDbBuilder.DefaultAccountKey
    };
    return string.Join(";", properties.Select(kvp => string.Join("=", kvp.Key, kvp.Value)));
}

Benefit

This method will make it easier to create connection strings used to build other containers which communicate with the cosmos container.

Alternatives

This connection string can be created using the code snippet proposed above, but results in duplicated code.

Would you like to help contributing this enhancement?

Yes

HofmeisterAn commented 10 months ago

I do not think this refers to the Cosmos DB Emulators module only. No module shares a method or property that resolves the internal connection string, base address, endpoint, or whatever it may be called. This heavily depends on the actual test configuration and how you put various containers together. Currently, we recommend using the network alias and manually configuring the connection, as we do, for instance, with the WebDrive module or in our Flyway example. Nevertheless, I comprehend that supporting something like this might slightly reduce the amount of necessary code and configuration.

Ideally, this method would work before the container is started so that it can be used to build other containers which rely on this connection string.

I do not understand. How would you interact with the container service before it starts?

goldsam commented 10 months ago

This heavily depends on the actual test configuration and how you put various containers together.

I respectfully disagree. You are either accessing a container from the host or from within a virtual container network. The former case has been addressed. The later case always relies on simply an internal network alias and port. The port is typically a fixed property of an image. This can be easily supported using my proposed method (and perhaps an overload that allows also specifying the port for services (no just cosmos) where the port can be changed via command line args or env var).

I do not understand. How would you interact with the container service before it starts?

Sorry if I was unclear. I am not proposing interacting with the container service before it starts, just the construction the internal network connection string. This relies only on the network alias (which is set on the builder via CosmosDbBuilder.WithNetworkAliases(...) prior to building the CosmosDbContainer) and the internal port which is a fixed property of the cosmos db image. Hence, it is possible to implement my proposed method such that it can be called on the a CosmosDbContainer instance prior to starting it. This is important, because then it can be used to builder other containers that subsequently rely on this internal connection string.

HofmeisterAn commented 10 months ago

I respectfully disagree. You are either accessing a container from the host or from within a virtual container network.

I was referring to the entire setup. Usually, it requires a bit more to set up (multiple containers, a network, etc.).

I have been thinking about this for some time now, given that developers occasionally ask for ways to override or configure the connection string. Perhaps there are similarities we can build upon. I was thinking about extending the builder API that receives some kind of connection string provider, capable of resolving a connection string for the test host - container and container - container communication. This would not only provide the right connections, but developers would also be able to override the default implementation and customize the connection string to suit their needs. Additionally, this could be used for abstraction, similar to what this PR requests.

goldsam commented 10 months ago

Would you have builders for each DB impl? Something like:

var someContainer = ...;

var connectionString = new CosmosDbConnectionStringBuilder()
    .WithInternalHost(someContainer)
    .Build();

var cosmos = new CosmosDbBuilder()
    .WithConnectionString(connectionString)
    .Build();

This would be wonderful 😊

HofmeisterAn commented 9 months ago

Would you have builders for each DB impl? Something like:

Yes, quite similar. I will close this issue. Let's continue the discussion here. Thanks for bringing the issue to my attention. If you think we should keep this issue still open, please do not hesitate to reopen it again.