Take each upstream's allow-list whole from the source that wins it [patch]

GitBranchStateCache.Tests/Tool/ConfigFileAllowListTests.cs

@@ -0,0 +1,129 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.GitBranchStateCache.Tests.Tool;
+
+using System.Text;
+using ktsu.GitBranchStateCache.Configuration;
+using ktsu.GitBranchStateCache.Tool;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
+
+/// <summary>
+/// A <c>--config</c> file layered over an <c>appsettings.json</c> in the working directory replaces each
+/// upstream's allow-list it names, rather than patching it by index (#120).
+/// </summary>
+[TestClass]
+public class ConfigFileAllowListTests
+{
+	private const string AppSettings = """
+		{
+		  "GitBranchStateCache": {
+		    "Upstreams": {
+		      "github": { "BaseUrl": "https://github.com", "Repositories": ["studio/sandbox.git", "studio/secret.git"] },
+		      "ado": { "BaseUrl": "https://dev.azure.com/org", "Repositories": ["project/_git/game", "other/_git/tools"] }
+		    }
+		  }
+		}
+		""";
+
+	private const string ConfigFile = """
+		{
+		  "GitBranchStateCache": {
+		    "Upstreams": {
+		      "GitHub": { "Repositories": ["studio/game.git"] }
+		    }
+		  }
+		}
+		""";
+
+	/// <summary>
+	/// Binds the options the way the tool does: the working directory's file, the <c>--config</c> file
+	/// over it, then the flags over both.
+	/// </summary>
+	private static GitBranchStateCacheOptions Bind(string[] sources, params string[] allows)
+	{
+		Assert.IsTrue(
+			Program.TryParseAllows(allows, out Dictionary<string, List<string>> allowLists, out string? invalid),
+			invalid);
+
+		ConfigurationBuilder builder = new();
+		List<MemoryStream> streams = [];
+		foreach (string source in sources)
+		{
+			MemoryStream stream = new(Encoding.UTF8.GetBytes(source));
+			streams.Add(stream);
+			builder.AddJsonStream(stream);
+		}
+
+		IConfigurationRoot configuration = builder.Build();
+		foreach (MemoryStream stream in streams)
+		{
+			stream.Dispose();
+		}
+
+		Dictionary<string, List<string>> resolved = Program.ResolveAllowLists(configuration, allowLists);
+
+		ServiceCollection services = new();
+		services.AddOptions<GitBranchStateCacheOptions>()
+			.Bind(configuration.GetSection(GitBranchStateCacheOptions.SectionName));
+		services.PostConfigure<GitBranchStateCacheOptions>(options => Program.ReplaceAllowLists(options, resolved));
+
+		using ServiceProvider provider = services.BuildServiceProvider();
+		return provider.GetRequiredService<IOptions<GitBranchStateCacheOptions>>().Value;
+	}
+
+	private static void AssertRepositories(GitBranchStateCacheOptions options, string upstream, params string[] expected) =>
+		CollectionAssert.AreEqual(expected, options.Upstreams[upstream].Repositories.ToArray());
+
+	[TestMethod]
+	public void ConfigFile_ShorterList_ReplacesTheAppSettingsList()
+	{
+		GitBranchStateCacheOptions options = Bind([AppSettings, ConfigFile]);
+
+		AssertRepositories(options, "github", "studio/game.git");
+	}
+
+	[TestMethod]
+	public void ConfigFile_LeavesAnUpstreamItDoesNotNameAsAppSettingsHasIt()
+	{
+		GitBranchStateCacheOptions options = Bind([AppSettings, ConfigFile]);
+
+		AssertRepositories(options, "ado", "project/_git/game", "other/_git/tools");
+	}
+
+	[TestMethod]
+	public void ConfigFile_KeepsTheBaseUrlFromAppSettings()
+	{
+		GitBranchStateCacheOptions options = Bind([AppSettings, ConfigFile]);
+
+		Assert.AreEqual("https://github.com", options.Upstreams["github"].BaseUrl?.ToString().TrimEnd('/'));
+	}
+
+	[TestMethod]
+	public void Allow_BeatsBothFiles()
+	{
+		GitBranchStateCacheOptions options = Bind([AppSettings, ConfigFile], "github=studio/engine.git");
+
+		AssertRepositories(options, "github", "studio/engine.git");
+	}
+
+	[TestMethod]
+	public void SingleSource_KeepsItsListInIndexOrder()
+	{
+		const string TwelveEntries = """
+			{ "GitBranchStateCache": { "Upstreams": { "github": { "Repositories": [
+			  "a/0.git", "a/1.git", "a/2.git", "a/3.git", "a/4.git", "a/5.git",
+			  "a/6.git", "a/7.git", "a/8.git", "a/9.git", "a/10.git", "a/11.git"
+			] } } } }
+			""";
+
+		GitBranchStateCacheOptions options = Bind([TwelveEntries]);
+
+		AssertRepositories(
+			options,
+			"github",
+			"a/0.git", "a/1.git", "a/2.git", "a/3.git", "a/4.git", "a/5.git",
+			"a/6.git", "a/7.git", "a/8.git", "a/9.git", "a/10.git", "a/11.git");
+	}
+}

GitBranchStateCache.Tool/Program.cs

@@ -153,10 +153,14 @@
 
 		ApplyDefaults(builder.Configuration, overrides);
 		builder.Configuration.AddInMemoryCollection(overrides);
 		builder.Configuration["Kestrel:Endpoints:Http:Url"] = $"http://*:{port}";
 
+		// Each upstream's list comes whole from one place: the --allow flags, else the highest-precedence
+		// source that sets it. Binding alone would merge the lists by index across sources.
+		Dictionary<string, List<string>> resolvedAllowLists = ResolveAllowLists(builder.Configuration, allowLists);
+
 		builder.Services.AddGitBranchStateCache(builder.Configuration);
-		builder.Services.PostConfigure<GitBranchStateCacheOptions>(options => ReplaceAllowLists(options, allowLists));
+		builder.Services.PostConfigure<GitBranchStateCacheOptions>(options => ReplaceAllowLists(options, resolvedAllowLists));
 
 		// Behind an ingress the request this service sees is not the one the client made. Nothing here
 		// builds a URL from the request, so this exists for the client address in the logs rather than
@@ -315,6 +319,76 @@
 		return true;
 	}
 
+	/// <summary>
+	/// Works out each upstream's allow-list as one value, taken whole from the source that wins it.
+	/// </summary>
+	/// <remarks>
+	/// Configuration merges arrays by index, so a <c>--config</c> file naming one repository over an
+	/// <c>appsettings.json</c> naming two would bind the file's repository and the second one from
+	/// <c>appsettings.json</c>, silently allowing a repository the operator never named in the file they
+	/// passed. Here the last source that sets any <c>Upstreams:{name}:Repositories</c> entry supplies the
+	/// whole list, and an <c>--allow</c> flag for the upstream beats every source.
+	/// </remarks>
+	/// <param name="configuration">The configuration the options are bound from, with every source added.</param>
+	/// <param name="flagAllowLists">The patterns given for each upstream with <c>--allow</c>.</param>
+	/// <returns>Each upstream's resolved list, for <see cref="ReplaceAllowLists"/>.</returns>
+	internal static Dictionary<string, List<string>> ResolveAllowLists(
+		IConfigurationRoot configuration,
+		IReadOnlyDictionary<string, List<string>> flagAllowLists)
+	{
+		Dictionary<string, List<string>> resolved = new(StringComparer.OrdinalIgnoreCase);
+		string upstreamsPath = $"{GitBranchStateCacheOptions.SectionName}:Upstreams";
+
+		foreach (IConfigurationSection upstream in configuration.GetSection(upstreamsPath).GetChildren())
+		{
+			string repositoriesPath = $"{upstreamsPath}:{upstream.Key}:Repositories";
+
+			foreach (IConfigurationProvider provider in configuration.Providers.Reverse())
+			{
+				List<string> indexes = [.. provider.GetChildKeys([], repositoriesPath).Distinct(StringComparer.OrdinalIgnoreCase)];
+				if (indexes.Count == 0)
+				{
+					continue;
+				}
+
+				// Array entries are keyed by their index; order them as the binder does, numerically.
+				indexes.Sort(CompareIndexes);
+
+				List<string> patterns = [];
+				foreach (string index in indexes)
+				{
+					if (provider.TryGet($"{repositoriesPath}:{index}", out string? pattern) && pattern is not null)
+					{
+						patterns.Add(pattern);
+					}
+				}
+
+				resolved[upstream.Key] = patterns;
+				break;
+			}
+		}
+
+		foreach ((string name, List<string> patterns) in flagAllowLists)
+		{
+			resolved[name] = patterns;
+		}
+
+		return resolved;
+	}
+
+	private static int CompareIndexes(string left, string right)
+	{
+		bool leftIsNumber = int.TryParse(left, System.Globalization.NumberStyles.None, System.Globalization.CultureInfo.InvariantCulture, out int leftNumber);
+		bool rightIsNumber = int.TryParse(right, System.Globalization.NumberStyles.None, System.Globalization.CultureInfo.InvariantCulture, out int rightNumber);
+		return (leftIsNumber, rightIsNumber) switch
+		{
+			(true, true) => leftNumber.CompareTo(rightNumber),
+			(true, false) => -1,
+			(false, true) => 1,
+			_ => string.CompareOrdinal(left, right),
+		};
+	}
+
 	/// <summary>
 	/// Makes each upstream named by <c>--allow</c> allow exactly the patterns given for it.
 	/// </summary>

README.md

@@ -70,7 +70,7 @@ gitbranchstatecache --upstream github=https://github.com --allow github=studio/g
                     --upstream ado=https://dev.azure.com/myorg --allow ado=myproject/_git/game
 ```
 
-`--allow` is required at least once per upstream and is also repeatable. For an upstream it names, the flags replace whatever list configuration gives that upstream, rather than adding to it; upstreams no flag names keep their configured list. Unlike `ktsu.GitLfsCache`, there is no pattern meaning every repository: every pattern must name at least one literal path segment. One request for a repository not on the list would clone a permanent mirror of it onto a shared volume, sized by the repository rather than by the request, that nothing ever evicts.
+`--allow` is required at least once per upstream and is also repeatable. For an upstream it names, the flags replace whatever list configuration gives that upstream, rather than adding to it; upstreams no flag names keep their configured list. Configuration sources replace a list the same way: when an `appsettings.json` in the working directory and the `--config` file both give an upstream `Repositories`, the `--config` file's list is used whole, and none of the `appsettings.json` entries are added to it. Unlike `ktsu.GitLfsCache`, there is no pattern meaning every repository: every pattern must name at least one literal path segment. One request for a repository not on the list would clone a permanent mirror of it onto a shared volume, sized by the repository rather than by the request, that nothing ever evicts.
 
 Patterns match case insensitively, because forge repository names are, and a pattern that fails only because someone typed `Studio` is a support ticket rather than a control. An allowed repository path is then reduced to lower case before anything is derived from it, so clients that disagree about casing still share one mirror, one fetch and one cached diff, and the volume holds one directory per repository whatever casing was used to ask for it. What is sent to the forge keeps the caller's spelling.