Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Emit database names in manifest. #98

Closed
wants to merge 3 commits into from
Closed

Emit database names in manifest. #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
304884e
Emit database names in manifest.
mitchdenny Oct 6, 2023
041e10d
Missing curlies.
mitchdenny Oct 6, 2023
fcc3f30
Update git ignore to not checkin aspire-manifest.json
mitchdenny Oct 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,4 @@ node_modules/

# vscode python env files
.env
samples/DevHost/aspire-manifest-new.json
30 changes: 27 additions & 3 deletions src/Aspire.Hosting/Postgres/PostgresContainerBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static IDistributedApplicationComponentBuilder<PostgresContainerComponent
{
var postgresContainer = new PostgresContainerComponent(name);
return builder.AddComponent(postgresContainer)
.WithAnnotation(new ManifestPublishingCallbackAnnotation(WritePostgresComponentToManifest))
.WithAnnotation(new ManifestPublishingCallbackAnnotation((writer, ct) => WritePostgresComponentToManifestAsync(postgresContainer, writer, ct)))
.WithAnnotation(new ServiceBindingAnnotation(ProtocolType.Tcp, port: port, containerPort: 5432)) // Internal port is always 5432.
.WithAnnotation(new ContainerImageAnnotation { Image = "postgres", Tag = "latest" })
.WithEnvironment("POSTGRES_HOST_AUTH_METHOD", "trust")
Expand All @@ -32,9 +32,25 @@ public static IDistributedApplicationComponentBuilder<PostgresContainerComponent
});
}

private static async Task WritePostgresComponentToManifest(Utf8JsonWriter jsonWriter, CancellationToken cancellationToken)
private static async Task WritePostgresComponentToManifestAsync(PostgresContainerComponent component, Utf8JsonWriter jsonWriter, CancellationToken cancellationToken)
{
jsonWriter.WriteString("type", "postgres.v1");

var databases = component.Annotations.OfType<PostgresDatabaseAnnotation>();
if (databases.Any())
{
jsonWriter.WriteStartObject("databases");

foreach (var database in databases)
{
jsonWriter.WriteStartObject(database.DatabaseName);
jsonWriter.WriteEndObject();
}

jsonWriter.WriteEndObject();

}

await jsonWriter.FlushAsync(cancellationToken).ConfigureAwait(false);
}

Expand All @@ -46,11 +62,19 @@ public static IDistributedApplicationComponentBuilder<T> WithPostgresDatabase<T>
{
connectionName = connectionName ?? postgresBuilder.Component.Name;

// We need to capture this here so that when we do manifest generation we know
// how to enumerate the databases.
if (databaseName != null && !postgresBuilder.Component.Annotations.OfType<PostgresDatabaseAnnotation>().Any(db => db.DatabaseName == databaseName))
{
postgresBuilder.WithAnnotation(new PostgresDatabaseAnnotation(databaseName));
}

return builder.WithEnvironment((context) =>
{
if (context.PublisherName == "manifest")
{
context.EnvironmentVariables[$"{ConnectionStringEnvironmentName}{connectionName}"] = $"{{{postgresBuilder.Component.Name}.connectionString}}";
var manifestConnectionString = databaseName == null ? $"{{{postgresBuilder.Component.Name}.connectionString}}" : $"{{{postgresBuilder.Component.Name}.databases.{databaseName}.connectionString}}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO - I wouldn't make it this complicated. I would think this should be good enough:

            "env": {
                "ConnectionStrings__postgres": "{postgres.connectionString};Database=catalogdb"
            },

That way there's no need for a new annotation, and no need to write the list of databases into the "type", "postgres.v1" object.

@ellismg - will string interpolation like I have above work correctly in azd? Get the connectionString from the postgres resource, and then append ;Database=catalogdb to it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ellismg - will string interpolation like I have above work correctly in azd? Get the connectionString from the postgres resource, and then append ;Database=catalogdb to it.

Not today - but we could make it work. The reason we didn't allow this was because we felt we'd need to come up with a way to escape the {. If we want to say you put in a literal { or } with a \{ or \} and escape a literal \ with \\, it would be easy enough to implement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two parts to making this emission work. One part is emitting the connection string, but the other part is emitting the list of databases on the postgres component.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the other part is emitting the list of databases on the postgres component.

why do we need that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that the thing that uses the manifest can create the database. You don't generally give apps that level of privs to the database server. In fact even DDL access is a bit too much.

context.EnvironmentVariables[$"{ConnectionStringEnvironmentName}{connectionName}"] = manifestConnectionString;
return;
}

Expand Down
11 changes: 11 additions & 0 deletions src/Aspire.Hosting/Postgres/PostgresDatabaseAnnotation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Aspire.Hosting.ApplicationModel;

namespace Aspire.Hosting.Postgres;

internal sealed class PostgresDatabaseAnnotation(string databaseName) : IDistributedApplicationComponentAnnotation
{
public string DatabaseName { get; } = databaseName;
}