search

dotnet / aspire

Tools, templates, and packages to accelerate building observable, production-ready apps
https://learn.microsoft.com/dotnet/aspire
MIT License
3.88k stars 469 forks source link

Aspire.Azure.Security.KeyVault component should be named Aspire.Azure.Security.KeyVault.Secrets #3930

Open pedropaulovc opened 6 months ago

pedropaulovc commented 6 months ago

Issue

Azure SDK provides 6 distinct clients to interact with the KeyVault data plane:

Currently Aspire only supports the first client but we should ensure that adding support for others is straightforward.

Expected

Following the pattern of how Aspire handles Azure Storage with its 3 data plane clients, the Aspire component that handles KeyVaultSecretClient should be named Aspire.Azure.Security.KeyVault.Secrets and the method to add the component to the DI container shold be named AspireKeyVaultSecretExtensions.AddAzureKeyVaultSecretClient

Actual

The Aspire component is named Aspire.Azure.Security.KeyVault and the method is named AspireKeyVaultExtensions.AddAzureKeyVaultClient

danmoseley commented 6 months ago

Any rename would presumably be something that would be breaking to do after GA..

@eerhardt who on Azure SDK have we worked with to make sure we're using the right naming /granularity for representing their functionality?

eerhardt commented 6 months ago

who on Azure SDK have we worked with to make sure we're using the right naming /granularity for representing their functionality?

@tg-msft @KrzysztofCwalina

I believe the original intention was to just create a single KeyVault component that would encompass Secrets, Certificates, and Keys. We just haven't done the latter 2 yet.

Azure.Security.KeyVault.Administration

I am unsure if we'd want "Administration" components in Aspire. Especially for now, I would just tell someone to use the Azure SDK library directly.

pedropaulovc commented 6 months ago

I believe the original intention was to just create a single KeyVault component that would encompass Secrets, Certificates, and Keys. We just haven't done the latter 2 yet.

How would the add component method name for Certificates and Keys clients be named? Keeping AddAzureKeyVaultClient, and adding AddAzureKeyVaultCertificatesClient and AddAzureKeyVaultKeysClient would be inconsistent.

I am unsure if we'd want "Administration" components in Aspire. Especially for now, I would just tell someone to use the Azure SDK library directly.

This will push the responsibility of keeping tracing, health checks, endpoint auth etc. aligned with other components that are managed by Aspire down to the library consumer, right? On a broader note, is it fair to assume that all Azure Aspire components could be eventually auto-generated from the Azure REST API specs?

tg-msft commented 6 months ago

Secrets are the primary use case for KV. We discussed this previously and were comfortable giving it the best/default name in Aspire.

We'll eventually be generating the baseline experience for Azure HTTP services for both Components and Hosting packages. We also want to be deliberate about how we roll out new libraries based on our ability to effectively support them, so please don't expect all of Azure in the next update.

martincostello commented 4 months ago

+1 to support for certificates. I have an app that uses both secrets and certificates so a one-liner to wire things up for certificate access the same as secrets would be welcome.

For now, I've just added this to my app as I know the connection string should just be the URI for passwordless authentication and assumes that DefaultAzureCredential will work:

services.AddSingleton((provider) =>
{
    var config = provider.GetRequiredService<IConfiguration>();
    var connectionString = config["ConnectionStrings:AzureKeyVault"];

    if (string.IsNullOrEmpty(connectionString) || !Uri.TryCreate(connectionString, UriKind.Absolute, out var vaultUri))
    {
        throw new InvalidOperationException("The specified Azure Key Vault connection string is invalid.");
    }

    return new CertificateClient(vaultUri, new DefaultAzureCredential());
});

I'd be happy to look at contributing (assuming it's mostly just-copy pasta of the secrets support) if an API design is approved.