Support multiple connection string formats

[davidfowl]

We might just start with node resources, but we should support a way to have resources support the nodejs connection information.

[IanWitham]

👍 I tried standing up my Strapi service with a Postgres database. I finally figured out that the connection string was in the wrong format. I'm reverting to docker-compose for now.

[davidfowl]

You can always manually wire up the various bits like you have to do with compose.

[FracturedCode]

I actually have a couple use cases for this. I can submit a PR, but I'd like to flesh out some details before submitting anything.

One possible solution I've thought of:

• A ConnectionStringFormat enum (SqlClient, Jdbc, etc)
• Change the IResourceWithConnectionString.GetConnectionString signature to accept nullable ConnectionStringFormat OR add a new overload
• modify implementations accordingly

Things I have questions about:

• How the manifest will capture this
• What the WithConnectionStringRedirection method is doing. Sounds interesting, no idea if it's relevant/can't tell exactly what it's doing.

[davidfowl]

This isn't something we'd take a PR for at this point (we're in aggressive stabilization mode at the moment). This requires more design (as you have captured), so we can iterate here with examples and maybe a design will fall out.

[davidfowl]

I think the first step here is to build samples with different connection string formats and see how ugly it is today so that we understand the scope of what needs to be changed.

PS: Aspire is optimized for .NET so I want to make sure we don't design something super generic that makes the mainline scenario worse 😄

[davidfowl]

@FracturedCode I spent some time thinking about your solution and:

A ConnectionStringFormat enum (SqlClient, Jdbc, etc)

This should be a string, not an enum.

Change the IResourceWithConnectionString.GetConnectionString signature to accept nullable ConnectionStringFormat

Yep but instead it would be GetConnectionStringExpression(string format)

modify implementations accordingly

You need an overload of WithReferene that supports specifying the format.

How the manifest will capture this

I think we'd add a connectionStrings key value pair on resources.

{
   "redis": {
       "connectingString": "..." 
        "connectingStrings": {
            "jdbc": ".."
        }
    }
}

[davidfowl]

This is something we're going to need to support for various reasons (for e.g. being able to get a kafka compatible connection string from an azure event hub)

[davidfowl]

@FracturedCode let me know if you want to iterate on the design.

[andi0b]

Manually formatting connection strings became much harder with the new ReferenceExpression.

From my experience there are so many different formats for connection strings, there must be a solution to customize them.

I came up with something like this (a very ugly hack that works), but that can't be the intended solution:

public static class PostgresExtensions
{
    public static ReferenceExpression GetConnectionUrl(this PostgresServerResource pg)
    {
        var userNameReference =
            pg.UserNameParameter is not null
                ? ReferenceExpression.Create($"{pg.UserNameParameter}")
                : ReferenceExpression.Create($"postgres");

        return ReferenceExpression.Create(
            $"postgresql://{userNameReference}:{pg.PasswordParameter}@{pg.PrimaryEndpoint.Property(EndpointProperty.Host)}:{pg.PrimaryEndpoint.Property(EndpointProperty.Port)}");
    }

    public static ReferenceExpression GetConnectionUrl(this PostgresDatabaseResource pg)
    {
        return ReferenceExpression.Create($"{pg.Parent.GetConnectionUrl()}/{pg.DatabaseName}");
    }
}

builder.AddNpmApp(...)
    .WithEnvironment("DB_CONNECTION_URL", () => db.Resource.GetConnectionUrl().GetValueAsync(CancellationToken.None).Result!)

[davidfowl]

What's wrong with the solution?

builder.AddNpmApp(...)
    .WithEnvironment(context => context.EnvironmentVariables["DB_CONNECTION_URL"] = db.Resource.GetConnectionUrl());

[andi0b]

builder.AddNpmApp(...)
    .WithEnvironment(context => context.EnvironmentVariables["DB_CONNECTION_URL"] = db.Resource.GetConnectionUrl());

That's a great suggestion, thanks! I couldn't figure that out on my own, is it documented somehow and I missed it, or is this just something that you need to know?

I didn't like that I had to replicate the logic of the protected PostgresServerResource.UserNameReference property and copy the value of the private PostgresServerResource.DefaultUserName field.

An overload for WithEnvironment that accepts a ReferenceExpression might make your solution more obvious. context.EnvironmentVariables is a Dictionary<string,object> and there is some digging though the source code needed to understand it can handle ReferenceExpressions.

[davidfowl]

I couldn't figure that out on my own, is it documented somehow and I missed it, or is this just something that you need to know?

We have no docs for this yet, it's coming in hot, but we will have them. @mitchdenny started to write an article on how to build custom resources and it includes some of this content https://github.com/dotnet/docs-aspire/pull/755.

Since I'm here, I will explain a bit behind how we ended up with this solution and why it matters for modeling connection strings and any other expression.

We're essentially building up a C# object model representation resources and they data they expose. For projects and containers, that includes endpoints, environment variables and arguments. The complexity here is that we want to preserve the object references so that we can have different behavior depending on the context. A ReferenceExpression is this object that lets you represent a formatted string that preserves the live object references. When you set this in the env, or args or anywhere else its supported, we can do interesting things like translate those object references into an IaC format (our manifest being one version of this).

I didn't like that I had to replicate the logic of the protected PostgresServerResource.UserNameReference property and copy the value of the private PostgresServerResource.DefaultUserName field.

These are the growing pains of a v1 framework. We'll be able to expose more API once we get a better understanding of what the common patterns are.

builder.AddNpmApp(...) .WithEnvironment(context => context.EnvironmentVariables["DB_CONNECTION_URL"] = db.Resource.GetConnectionUrl()); That's a great suggestion, thanks! I couldn't figure that out on my own, is it documented somehow and I missed it, or is this just something that you need to know?

I didn't like that I had to replicate the logic of the protected PostgresServerResource.UserNameReference property and copy the value of the private PostgresServerResource.DefaultUserName field.

An overload for WithEnvironment that accepts a ReferenceExpression might make your solution more obvious. context.EnvironmentVariables is a Dictionary<string,object> and there is some digging though the source code needed to understand it can handle ReferenceExpressions.

Agree, would you like to submit a PR?

We have this overload but it's inadequate. You can follow the pattern here.

Thanks for the feedback!

cc @IEvangelist

[mitchdenny]

Moved to backlog. We need to decide if/when we want to do this. I think we need some way of mutating the string expression when we WithReference(...). But maybe WithEnvironment is enough?

[andi0b]

@mitchdenny I think WithEnvironment is okay, as long as there is an intuitive way to assembly the connection string in code. From my experience there are unlimited different connection string formats required by 3rd party software.

Many require multiple environment variables (hostname, dbname, username, password, ...) instead of a single connection string. There are even hybrid approaches, for example keycloak:

KC_DB_URL=jdbc:postgresql://host/dbname
KC_DB_USERNAME=username
KC_DB_PASSWORD=password

The .NET connection string format is a good default, but it should be trivial to "escape" to any other required format.

[FracturedCode]

I iterated a bit on @andi0b's code and came up with this which includes container support:

public static class PostgresExtensions
{

    private static ReferenceExpression getConnectionUrl(this PostgresDatabaseResource db, bool isInContainer)
    {
        var pg = db.Parent;
        var userNameReference = pg.UserNameParameter is not null
            ? ReferenceExpression.Create($"{pg.UserNameParameter}")
            : ReferenceExpression.Create($"postgres");

        // WARNING: this will mess up the manifest for the containers
        var host = isInContainer
            ? ReferenceExpression.Create($"host.docker.internal")
            : ReferenceExpression.Create($"{pg.PrimaryEndpoint.Property(EndpointProperty.Host)}");

        return ReferenceExpression.Create(
            $"postgresql://{userNameReference}:{pg.PasswordParameter}@{host}:{pg.PrimaryEndpoint.Property(EndpointProperty.Port)}/{db.DatabaseName}");
    }

    public static IResourceBuilder<T> WithReference<T>(
        this IResourceBuilder<T> builder, IResourceBuilder<PostgresDatabaseResource> db,
        ConnectionStringType connectionStringType = ConnectionStringType.Npgsql,
        string? connectionStringName = null
    ) where T : IResourceWithEnvironment
    {
        var connectionStringExpression = connectionStringType switch
        {
            ConnectionStringType.Uri => db.Resource.getConnectionUrl(builder is IResourceBuilder<ContainerResource>),
            ConnectionStringType.Npgsql => db.Resource.ConnectionStringExpression,
            _ => throw new NotSupportedException()
        };
        return builder.WithEnvironment(connectionStringName ?? $"connectionString__{db.Resource.Name}", connectionStringExpression);
    }
}

I did read the feedback about not using an enum and others, but I was selfishly making this for my own use case.

Consumption looks like this:

var infisicalDb = builder.AddPostgres("postgres", null, pgPw)
    .WithEnvironment("POSTGRES_DB", infisicalDbName)
    .AddDatabase(infisicalDbName);
builder.AddContainer("infisical", "infisical/infisical", "v0.83.0-postgres")
    .WithReference(infisicalDb, ConnectionStringType.Uri, "DB_CONNECTION_URI");

This almost works perfect, except that you'll notice the WARNING comment. This workaround means the manifest indicates the string host.docker.internal instead of the reference to the pg host. Why was the workaround needed? If the ReferenceExpression to the pg host is used, it resolves to localhost which doesn't work with the container's networking model.

This got me wondering how this problem is handled in Aspire. How do the connection strings in containers end up using host.docker.internal instead of localhost while still outputting the correct manifest contents? Here is how the default connection string is constructed in PostgresServerResource:

private ReferenceExpression ConnectionString =>
    ReferenceExpression.Create($"Host={PrimaryEndpoint.Property(EndpointProperty.Host)};Port={PrimaryEndpoint.Property(EndpointProperty.Port)};Username={UserNameReference};Password={PasswordParameter}");

Notice how it does use the reference expression for the host instead of the string literal. This is what we would want in an ideal world. Could it be that when the ReferenceExpression is resolved that the consumer of the connection string can be specified thus altering the output accordingly? No, it turns out the connection string still resolves to localhost even when the consumer intends it for a container.

DCP's ApplicationExecutor.GetValue contains the answer. It does a string.Replace on the connection string:

if (value is not null && isContainer && valueProvider is ConnectionStringReference or EndpointReference or HostUrl)
{
    // If the value is a connection string or endpoint reference, we need to replace localhost with the container host.
    return ReplaceLocalhostWithContainerHost(value);
}

The if check ends up explaining why our getConnectionUrl method ends up being localhost while the built in connection string ends up being host.docker.internal. The traditional WithReference constructs a ConnectionStringReference while the getConnectionUrl is providing a plain old ReferenceExpression.

A couple solutions off the top of my head would be overloading ConnectionStringReference or creating a new type, but those are very small picture solutions.