Why do you need both?

Redis uses argument based config for TLS, which was screwing up the test this is for (runs tail on /dev/null instead of redis); turned them off so they wouldn’t try to contribute arguments.

Are we turning default https on for everything or will it be opt in based on the endpoints added?

Current design turns it on by default (and adds TLS/HTTPS endpoints automatically if a user doesn’t disable the feature). In the case of uvicorn, the server only supports a single endpoint, so I have to “upgrade” the existing endpoint to an https scheme.

Seems like a lot of this applies to any .Net/Kestrel container. Could this be promoted to some kind of extension method that I could use on my own dotnet containers, rather than needing to re-implement what this block does?

Initial release I'll keep this specific to YARP, but after it's been out in the wild and we've had a chance to identify and fix any issues that crop up we can consider adding something like a WithKestrelCertificateKeyPairConfiguration() extension method for containers.

Should this also check if the private key is exportable? (Which is presumably required to be able to inject it into a container).

For the dev cert this shouldn't be an issue as it's always added exportable, but I'll open up a follow-up PR to double check.

Meant to comment this on the dev certs service comment. I'll tackle both in a follow-up PR though as the defaults should work fine when using a dev cert and I'd expect a user to provide an exportable certificate when using custom configuration.

Should this also check that the key is exportable?

Is WithCertificateKeyPair only for TLS certs, or can it be used to inject other certs. If the latter, what if I want to inject my custom certs, but not the dev cert? If hte former, should these methods be renamed With(out)TlsCertificateKeyPair()

With these being ReferenceExpressions, how would I put these into a config file. e.g. for sql server, which seems to require TLS be configured via /var/opt/mssql/mssql.conf

[network]
forceencryption = 1
tlscert = {CertificatePath}
tlskey = {KeyPath}

Evaluate the reference expression

Just tried this out and hit a few issues

.WithDeveloperCertificateKeyPair()
.WithCertificateKeyPairConfiguration(async ctx =>
{
    builder.CreateResourceBuilder((SqlServerServerResource)ctx.Resource)
    .WithContainerFiles("/var/opt/mssql/conf/", [
        new ContainerFile {
            Name = "mssql.conf",
            Contents = $"""
                        [network]
                        forceencryption = 1
                        tlscert = {await ctx.CertificatePath.GetValueAsync(ctx.CancellationToken).ConfigureAwait(false)}
                        tlskey = {await ctx.KeyPath.GetValueAsync(ctx.CancellationToken).ConfigureAwait(false)}
                        """
        }
    ]);

    //Need to set an env var or arg
    //ctx.EnvironmentVariables["TLSFAKE"] = true;
});

1. This callback WithCertificateKeyPairConfiguration is executed after container files are built, so the WithContainerFiles call above does nothing. After swapping the order of BuildCreateFilesAsync and BuildContainerCertificateKeyPairAsync in DCP executor, I did get this to work. Although given that the current implementaiton does some special handling of env vars and args, I'm sort of assuming this "fix" causes a bunch of other problems
2. Because I set no env var or args, Aspire incorrectly logs "Resource 'sql1' does not have certificate key pair configuration defined. No TLS key pair override will be applied."
3. The builder.CreateResourceBuilder((SqlServerServerResource)ctx.Resource) syntax is a bit rough. is there a better way to add a container file?

See https://github.com/danegsta/aspire/compare/dev/danegsta/certificateKeyPair...afscrome:sqltls?expand=1

Main concern is that modifying annotations late (after BeforeStartEvent) isn't necessarily safe depending as there could be some other operation iterating over the annotations collection, hence why this (and the certificate trust API) use callback contexts with the same conventions as WithEnvironment and WithArgs.

For supporting applying custom file based configuration, it feels like making the certificate properties (both the trusted certificates and keypair related properties) available in the context of the WithContainerFiles callback may be the best approach to supporting that scenario as it's quite possible (if not likely) that the the config file would need access to more than just the TLS certs.

So something like:

.WithContainerFiles("/var/opt/mssql/conf/", (ctx, cancellationToken) => {
    return Task.From<IEnumerable<ContainerFileSystemItem>>(new List<ContainerFileSystemItem>() {
        new ContainerFile {
            Name = "mssql.conf",
            Contents = $"""
                        [network]
                        forceencryption = 1
                        tlscert = {await ctx.ServerAuthenticationCertificatePath.GetValueAsync(ctx.CancellationToken).ConfigureAwait(false)}
                        tlskey = {await ctx.ServerAuthenticationKeyPath.GetValueAsync(ctx.CancellationToken).ConfigureAwait(false)}
                        """
        },
    });
});

As for that log entry if there's no config to apply; I've got a separate PR where I'm refactoring how the set of config callbacks are run that'll need to be updated to cover these new APIs as well and I'd like remove it there as it wouldn't make sense anymore.

Adding the values within WithContainerFiles would make sense, although it would probably make more sense to provide the values as string there rather than as a ReferenceExpression - I'm not sure you could use a ReferenceExpression meaningfully more than a string in that context.

The WithContainerFiles changes will likely come in a follow-up PR (no guarantees that PR would make it into 13.1 as we're trying for a pretty quick turnaround, but I'll ping you once it's open so you can try it out).

FYI, I've now got tls cert injection "working" with SQL Server using TLS. It's not mergeable as it needs some test work and some workarounds for the fact that SQL server requires the use of 127.0.0.1 rather than localhost. But what I have should be good enough to prove the cert injection API works with the follow up PR.

https://github.com/danegsta/aspire/compare/dev/danegsta/certificateKeyPair...afscrome:sqltls?expand=1

Definitely appreciate you taking a look at how this would need to work with SQL Server; I had to pick an arbitrary subset of integrations to implement in the initial release, so having more eyes looking at other integrations and trying things out is extremely helpful.

I expect we'll need to figure out a way to make 127.0.0.1 work for more than just SQL Server; I started looking at supporting TLS in Azurite, but some legacy decisions in the Azure Storage SDK mean that we need a certificate that's valid for either 127.0.0.1 or *.blob.localhost style domains (the SDK special cases IP addresses and specific ports for local access, any hostname is evaluated as a custom storage domain endpoint, with no allowance for localhost addresses).