dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.09k stars 4.7k forks source link

[API Proposal]: Allow multiple UserSecretIds for shared secret accross mutliple local repositories #89761

Open tebeco opened 1 year ago

tebeco commented 1 year ago

Background and motivation

This affects teams working in organizations which works on multiple (micro or not) services.

When working with multiple Project/Api, the chance is hight that most services will share the same monitoring cluster / cache / SignalR / etc.... The result is that

Hopefully, the Configuration Api handle adding multiple source of configurations for that. The limitation today is that UserSecrets can only ever loaded once which make it impossible to re-use shared key:

When a key is edited:

API Proposal

Unsure if we should add UserSecretsIds (with an s) in addition to UserSecretsIds. I'm guessing both are probably possible

[AttributeUsage(AttributeTargets.Assembly, Inherited = false, AllowMultiple = false)]
public class UserSecretsIdAttribute : Attribute
{
    public UserSecretsIdAttribute(string userSecretId)
    {
        // it's probably the easiest way to integrate with msbuild
        // Trim empty entries
        // De-dup
        UserSecretsIds = userSecretId
            .Split(new string[] { ";" }, StringSplitOptions.RemoveEmptyEntries)
            .Distinct()
            .ToArray();
    }

    public string[] UserSecretsIds { get; }
}

Will load foo and then bar. Just like all `Configuration provider the order matters, nothing changes here

API Usage

single:

<PropertyGroup>
  <UserSecretsId>foo</UserSecretsId>
<PropertyGroup>

multiple:

<PropertyGroup>
  <UserSecretsId>foo;bar</UserSecretsId>
<PropertyGroup>

Alternative Designs

It's unclear for me what's the best solution here between:

Sticking with the proposal would allow to not change this part of the code: .\src\libraries\Microsoft.Extensions.Configuration.UserSecrets\src\buildTransitive\Microsoft.Extensions.Configuration.UserSecrets.targets

[assembly:Microsoft.Extensions.Configuration.UserSecrets.UserSecretsIdAttribute(UserSecretsId)]
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

  <PropertyGroup>
    <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
    <GenerateUserSecretsAttribute Condition="'$(GenerateUserSecretsAttribute)'==''">true</GenerateUserSecretsAttribute>
  </PropertyGroup>

  <ItemGroup Condition=" '$(UserSecretsId)' != '' AND '$(GenerateUserSecretsAttribute)' != 'false' ">
    <AssemblyAttribute Include="Microsoft.Extensions.Configuration.UserSecrets.UserSecretsIdAttribute">
      <_Parameter1>$(UserSecretsId.Trim())</_Parameter1>
    </AssemblyAttribute>
  </ItemGroup>
</Project>

Risks

If MsBuild pluralization is asked, then we would need to consider that consumers could have conflict such as:

<UserSecretsId>foo</UserSecretsId>
<UserSecretsIds>foo</UserSecretsIds>
<UserSecretsId>foo</UserSecretsId>
<UserSecretsIds>bar</UserSecretsIds>
<UserSecretsId>foo</UserSecretsId>
<UserSecretsIds>foo;foo</UserSecretsIds>
ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-extensions-configuration See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation This affects teams working in organizations which works on multiple (micro or not) services. When working with multiple Project/Api, the chance is hight that most services will share the same monitoring cluster / cache / SignalR / etc.... The result is that - it's quite frequent to have common / shared secret value between `ServiceA` and `ServiceB` while clone few douzen of repo at work. - It's possible that some secret require an override, like testing `ServiceA` new Redis code in a local redis container etc ... - At the same time `ServiceA` might have key specific to `ServiceA` which are not desired to be loaded or could conflict in `ServiceB` Hopefully, the `Configuration` Api handle adding multiple source of configurations for that. The limitation today is that `UserSecrets` can only ever loaded once which make it impossible to re-use shared key: * Sharing the secret id: possible conflict / no local override * Manually duplicating file locally on disk When a key is edited: * Open all repo * Find the secret id * Find the file, * Search the key, + edit all manually ### API Proposal Unsure if we should add `UserSecretsIds` (with an `s`) in addition to `UserSecretsIds`. I'm guessing both are probably possible ```csharp [AttributeUsage(AttributeTargets.Assembly, Inherited = false, AllowMultiple = false)] public class UserSecretsIdAttribute : Attribute { public UserSecretsIdAttribute(string userSecretId) { // it's probably the easiest way to integrate with msbuild // Trim empty entries // De-dup UserSecretsIds = userSecretId .Split(new string[] { ";" }, StringSplitOptions.RemoveEmptyEntries) .Distinct() .ToArray(); } public string[] UserSecretsIds { get; } } ``` Will load `foo` and then `bar`. Just like all `Configuration provider the order matters, nothing changes here ### API Usage single: ```xml foo ``` multiple: ```xml foo;bar ``` ### Alternative Designs It's unclear for me what's the best solution here between: * Change existing `UserSecretsIdAttribute` from `string` to `string[]` in `property` only and split + dedup in ctor * Change existing `UserSecretsIdAttribute` from `string` to `string[]` in `ctor` + `property` Impact: will create clear/direct breaking code at build time * Adding a new `UserSecretsIdsAttribute` Impact: might create runtime bug on existing code/third part not updating how to list attributes * Changing `AllowMultiple = false` to `AllowMultiple = true` on existing `UserSecretsIdsAttribute` Impact: might create runtime bug on existing code/third part not updating how to list attributes Sticking with the proposal would allow to not change this part of the code: `.\src\libraries\Microsoft.Extensions.Configuration.UserSecrets\src\buildTransitive\Microsoft.Extensions.Configuration.UserSecrets.targets` ```cs [assembly:Microsoft.Extensions.Configuration.UserSecrets.UserSecretsIdAttribute(UserSecretsId)] ``` ```xml $(MSBuildAllProjects);$(MSBuildThisFileFullPath) true <_Parameter1>$(UserSecretsId.Trim()) ``` ### Risks If MsBuild pluralization is asked, then we would need to consider that consumers could have conflict such as: ```xml foo foo ``` ```xml foo bar ``` ```xml foo foo;foo ```
Author: tebeco
Assignees: -
Labels: `api-suggestion`, `untriaged`, `area-Extensions-Configuration`
Milestone: -