Kralizek / AWSSecretsManagerConfigurationExtensions

This repository contains a provider for Microsoft.Extensions.Configuration that retrieves secrets stored in AWS Secrets Manager.
MIT License
231 stars 44 forks source link

New API model #80

Open Kralizek opened 1 year ago

Kralizek commented 1 year ago

This PR introduces a new API model for registering and configuring the provider.

avalara-stephen-hickey commented 1 year ago

I like having the credentials and region moved to the options.

I don't think I care for the examples in Sample8. For instance, if someone wants to make changes to the options they are passing to .UseConfiguration, they should just do it before passing it to the library.

// Sample8
var configuration = configurationBuilder.Build();
var options = configuration.GetSection("SecretsManager").GetSecretsManagerOptions()
configurationBuilder.AddSecretsManager(configure =>
{
    configure.UseConfiguration(configuration.GetSection("SecretsManager").GetSecretsManagerOptions());
    configure.Configure(o => o.AcceptedSecretArns.Add("arn:example:03"));
    ...
});

// Idea
var configuration = configurationBuilder.Build();
var options = configuration.GetSection("SecretsManager").GetSecretsManagerOptions()
options.AcceptedSecretArns.Add("arn:example:03");

configurationBuilder.AddSecretsManager(configure =>
{
    configure.UseConfiguration(options);
});

I think the options inside another options class is also a bit confusing.

And if you go that way, I don't know why a user doesn't just build the entire options class themselves and pass it to the library.

var configuration = configurationBuilder.Build();
var options = configuration.GetSection("SecretsManager").GetSecretsManagerOptions()
options.AcceptedSecretArns.Add("arn:example:03");
options.AwsOptions = configuration.GetSection("SecretsManagerProfile").GetAWSOptions();

// The method signature would look like
// `.AddSecretsManager(SecretsManagerConfigurationProviderOptions = null, Action<SecretsManagerConfigurationProviderOptions>? configure = null)`
// Or you could have several overloads, whichever you prefer.
configurationBuilder.AddSecretsManager(options, configure =>
{
    configure.AcceptedSecretArns.Add("arn:example:03");
});

That way, users who want to pull from configuration and who want to configure it programmatically both look very similar. Or even users who want part of it in configuration and part of it done programmatically.

Kralizek commented 1 year ago

I don't know why a user doesn't just build the entire options class themselves and pass it to the library.

@avalara-stephen-hickey Thank you for your comment. It inspired me to greatly simplify the API.

I'm not sure if I went too far and cut too much now. I still have to decide the final name for the options classes and I want to rework on the scopes marking more stuff as internal and possibly sealed.