Take AWS options from appsettings

[kpenergy]

This is a nice little library, but one issue that I'm having is that it doesnt take AWS options from appsettings. What are your thoughts on supporting this?

For example, if I have the following in my appsettings file, I'd want this information to be used when creating the Secrets Manager client:

{
    "AWS": {
        "Region": "eu-west-1",
        "Profile": "dev",
    }
}

At the moment, it only seems to use the information in the credentials file, or if the AWS_PROFILE or AWS_REGION environment variable are present, it'll use the information from there.

The AWS .NET Configuration Extension for Systems Manager has an AddSystemsManager method which builds the IConfigurationBuilder and fetches the AWSOptions from it by IConfiguration.GetAWSOptions(), which allows these options to come from appsettings. I'm wondering if it would be possible to support something similar in this library.

[Kralizek]

Hi @kpenergy

I confess I haven't tried too much but my understanding is that this library main piece is a configuration provider and therefore there is no configuration available at the time of execution.

[kpenergy]

I think the configuration is available at execution time (providing it has been added to the IConfigurationBuilder that's extended in the AddSecretsManager method).

I think we'd just need to update your SecretsManagerExtensions.AddSecretsManager method to do something similar to what's done in the AwsOptionsProvider.GetAwsOptions method ,e.g:

var config = builder.Build();
var newOptions = config.GetAWSOptions();
string AwsOptionsConfigurationKey = "AWS_CONFIGBUILDER_AWSOPTIONS";
if (builder.Properties.ContainsKey(AwsOptionsConfigurationKey))
{
    builder.Properties[AwsOptionsConfigurationKey] = newOptions;
}
else
{
    builder.Properties.Add(AwsOptionsConfigurationKey, newOptions);
}

I'll leave this ticket open and will investigate further over the next couple of days.

[Kralizek]

@kpenergy if that works, we could also solve this one #57

[devklick]

I've made a start on this an have the basic POC working, allowing AWS configuration (the region & profile) and the secrets manager configuration (ARN list, filters, polling interval) to be read from a configuration section.

The idea is that we can support something like this in our appsettings file:

{
  "AWS": {
    "Region": "eu-west-1",
    "Profile": "dev"
  },
  "MySecretsManager": {
    "PollingIntervalInSeconds": 60,
    "AcceptedSecretArns": [
      "arn:blah:1",
      "arn:blah:2"
    ],
    "ListSecretsFilters": [
      {
        "Key": "Name",
        "Values": [
          "Value1",
          "Value2"
        ]
      }
    ]
  }
}

And to use this configuration, we just specify the relevant option when configuring the secrets manger:

builder.AddSecretsManager(configurator: o =>
{
    o.ReadFromConfigSection("MySecretsManager");
    // We can still specify options here, too...
    o.KeyGenerator = (entry, key) => key.ToUpper();
});

If some options are specified in both the code and in the appsettings file, the code will win. We'll try and combine them where possible (e.g. if AcceptedSecretArns are provided in code and JSON, we'll combine these), however if they cannot be combined (e.g. if the PollingInterval timespan is set in code and the PollingIntervalInSeconds is set in JSON), we'll use the value from code.

Note that even though we can specify the config section name when calling ReadFromConfigSection, the AWS options (region & profile) will still be taken from the standard AWS config section. We could look to allow the region & profile to be specified in the custom config section, but I'm not sure there's much value in that.

I still need to do some tidying up and spend more time on testing, but I should have it done by the end of week. Feel free to add comments here if there's any questions or suggestions in the meantime.

[devklick]

One thing I forgot to mention is that I've introduced two new package dependencies as part of this work:

• AWSSDK.Extensions.NETCore.Setup This is used for getting the AWS config section, by caling IConfiguration.GetAWSOptions();
• Microsoft.Extensions.Configuration.Binder This is used for getting the custom config section, by calling IConfiguration.GetSection("MySection").Get<MyClass>();

These are both fairly small packages, totalling around 42kb, so I'm not overly worried about these dependencies increasing the size of the SecretsManger library. But if you'd rather avoid these dependencies, we could just bind the configuration to the classes manually (reinventing the wheel, but may be worth it).

[Kralizek]

Thanks for trying this :)

I like the approach you're taking. I just wonder if AcceptedArns should be merged or replaced. I can see the value of both approaches.

As for the additional dependencies, I don't mind the Microsoft one but I'd avoid, if feasible, pulling in the AWS one. It's not much about the weight of the packages, rather the semantic behind this dependency for the users of this library.

[kpenergy]

Good challenge RE merge or replace.

One of the main benefits that I see of using JSON config files to specify AcceptedArns is having a different list for different environments. However you may have some ARNs which are relevant in all environments. Personally, I would probably write the common ARNs once in code rather than duplicate in multiple JSON files, so I'd rather they were combined. But that's just based on my use case.

What you say about the AWS dependency makes sense, and it's easy to get rid of (keeping the Microsoft one makes it easy to get rid of the AWS one).

[Kralizek]

Are @devklick and @kpenergy the same person? 🤔

[kpenergy]

Yeah sorry for confusion, personal and work account :)

[devklick]

I've been really struggling writing unit tests that cover this new functionality. Mocking the various IConfiguration* interfaces to return the appropriate values, for the SUT to then bind these to the relevant object, gets overly complex. It would be simple if the bind functionality (in this case, we call the IConfigurationSection.Get<T>() method) was not an extension method and could be mocked.

I've pushed up the latest code to feature/58-aws-options-from-appsettings, which contains the functionality that takes configuration from appsettings, a new example application to demonstrate usage, and an updated README.

While I do intend to come back to this and finish off with unit tests, I'm not likely to do so any time soon due to unexpected changes in my circumstances.

[Kralizek]

@devklick thanks for your effort. Usually when I have to test stuff that uses Microsoft.Extensions.Configuration and Microsoft.Extensions.DependencyInjection I end up using the concrete types rather than mocking the interfaces. I am aware it pushes towards integration tests but I prefer that to having complicated but "pure" unit tests.

When it comes to Configuration, I rely on the in-memory provider or the AddObject library I have here.

As for your work in progress, would you mind pushing your branch as a draft PR?

[devklick]

The problem I was hitting when using the concrete type for configuration was verifying that the SecretsManagerConfigurationSource had been added (and the contents of course). We cant hook into the IConfigurationBuilder.Add() method to verify what was added. I figured we could maybe build the concrete builder and try to fetch the SecretsManagerConfigurationSource, but doing so obviously builds the config source, resulting in the SecretsManagerConfigurationProvider being loaded, resulting in calls off to AWS to fetch. We could just catch and ignore the error - the scope of these tests doesnt care about the functionality that fetches secrets - but this just feels like an extremely messy way to test something.

It's deffo possible with mocks, it just requires a lot of mocking and will be messy.

I really like your AddObject extension. It's far nicer than constructing a dictionary manually to pass to AddInMemoryCollection.

I've created a PR for the changes I've got so far.