Make --allow replace an upstream's configured allow-list

GitBranchStateCache.Tests/GitBranchStateCache.Tests.csproj

@@ -30,5 +30,6 @@
 
   <ItemGroup>
     <ProjectReference Include="..\GitBranchStateCache\GitBranchStateCache.csproj" />
+    <ProjectReference Include="..\GitBranchStateCache.Tool\GitBranchStateCache.Tool.csproj" />
   </ItemGroup>
 </Project>

GitBranchStateCache.Tests/Tool/AllowFlagTests.cs

@@ -0,0 +1,107 @@
+// 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;
+
+[TestClass]
+public class AllowFlagTests
+{
+	private const string ConfigFile = """
+		{
+		  "GitBranchStateCache": {
+		    "Upstreams": {
+		      "github": { "BaseUrl": "https://github.com", "Repositories": ["studio/game.git", "studio/tools.git"] },
+		      "ado": { "BaseUrl": "https://dev.azure.com/org", "Repositories": ["project/_git/game", "other/_git/tools"] }
+		    }
+		  }
+		}
+		""";
+
+	/// <summary>
+	/// Binds the options the way the tool does: a configuration file, then the flags over it.
+	/// </summary>
+	private static GitBranchStateCacheOptions Bind(params string[] allows)
+	{
+		Assert.IsTrue(
+			Program.TryParseAllows(allows, out Dictionary<string, List<string>> allowLists, out string? invalid),
+			invalid);
+
+		using MemoryStream file = new(Encoding.UTF8.GetBytes(ConfigFile));
+		IConfiguration configuration = new ConfigurationBuilder().AddJsonStream(file).Build();
+
+		ServiceCollection services = new();
+		services.AddOptions<GitBranchStateCacheOptions>()
+			.Bind(configuration.GetSection(GitBranchStateCacheOptions.SectionName));
+		services.PostConfigure<GitBranchStateCacheOptions>(options => Program.ReplaceAllowLists(options, allowLists));
+
+		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 Allow_FewerPatternsThanConfigured_ReplacesTheConfiguredList()
+	{
+		GitBranchStateCacheOptions options = Bind("github=studio/engine.git");
+
+		AssertRepositories(options, "github", "studio/engine.git");
+	}
+
+	[TestMethod]
+	public void Allow_AsManyPatternsAsConfigured_ReplacesTheConfiguredList()
+	{
+		GitBranchStateCacheOptions options = Bind("github=a/one.git", "github=b/two.git");
+
+		AssertRepositories(options, "github", "a/one.git", "b/two.git");
+	}
+
+	[TestMethod]
+	public void Allow_Repeated_KeepsEveryPatternInOrder()
+	{
+		GitBranchStateCacheOptions options = Bind("github=a/one.git", "GitHub=b/two.git", "github=c/three.git");
+
+		AssertRepositories(options, "github", "a/one.git", "b/two.git", "c/three.git");
+	}
+
+	[TestMethod]
+	public void Allow_ForOneUpstream_LeavesTheOthersConfiguredList()
+	{
+		GitBranchStateCacheOptions options = Bind("github=studio/engine.git");
+
+		AssertRepositories(options, "ado", "project/_git/game", "other/_git/tools");
+	}
+
+	[TestMethod]
+	public void Allow_ForAnUnconfiguredUpstream_AddsItWithThoseRepositories()
+	{
+		GitBranchStateCacheOptions options = Bind("gitlab=team/app.git");
+
+		AssertRepositories(options, "gitlab", "team/app.git");
+	}
+
+	[TestMethod]
+	public void NoAllow_KeepsTheConfiguredList()
+	{
+		GitBranchStateCacheOptions options = Bind();
+
+		AssertRepositories(options, "github", "studio/game.git", "studio/tools.git");
+	}
+
+	[TestMethod]
+	[DataRow("github")]
+	[DataRow("=studio/game.git")]
+	[DataRow("github=")]
+	public void TryParseAllows_Malformed_ReportsTheEntry(string entry)
+	{
+		Assert.IsFalse(Program.TryParseAllows([entry], out _, out string? invalid));
+		Assert.AreEqual(entry, invalid);
+	}
+}

GitBranchStateCache.Tool/Program.cs

@@ -4,6 +4,7 @@
 
 using System.CommandLine;
 using ktsu.Essentials;
+using ktsu.GitBranchStateCache.Configuration;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.HttpOverrides;
 using Microsoft.Extensions.Configuration;
@@ -58,7 +59,7 @@
 		Option<string[]> allow = new("--allow", "-a")
 		{
 			Description =
-				"A repository this upstream may mirror, as name=pattern, for example github=studio/game.git. Repeatable. Required at least once per upstream, and every pattern must name a literal path segment.",
+				"A repository this upstream may mirror, as name=pattern, for example github=studio/game.git. Repeatable. Required at least once per upstream, and every pattern must name a literal path segment. Replaces, rather than adds to, the list configuration gives the upstream it names.",
 			AllowMultipleArgumentsPerToken = false,
 		};
 
@@ -108,14 +109,18 @@
 					.ConfigureAwait(false);
 			}
 
-			if (!TryApplyAllows(parseResult.GetValue(allow), overrides, out string? invalidAllow))
+			if (!TryParseAllows(
+				parseResult.GetValue(allow),
+				out Dictionary<string, List<string>> allowLists,
+				out string? invalidAllow))
 			{
 				return await FailAsync(
 					$"'{invalidAllow}' is not a valid allow entry. Use name=pattern, for example github=studio/game.git.")
 					.ConfigureAwait(false);
 			}
 
-			return await RunAsync(overrides, listenPort, configPath, cancellationToken).ConfigureAwait(false);
+			return await RunAsync(overrides, allowLists, listenPort, configPath, cancellationToken)
+				.ConfigureAwait(false);
 		});
 
 		return await root.Parse(args)
@@ -125,6 +130,7 @@
 
 	private static async Task<int> RunAsync(
 		Dictionary<string, string?> overrides,
+		Dictionary<string, List<string>> allowLists,
 		int port,
 		string? configPath,
 		CancellationToken cancellationToken)
@@ -147,9 +153,10 @@
 
 		ApplyDefaults(builder.Configuration, overrides);
 		builder.Configuration.AddInMemoryCollection(overrides);
 		builder.Configuration["Kestrel:Endpoints:Http:Url"] = $"http://*:{port}";
 
 		builder.Services.AddGitBranchStateCache(builder.Configuration);
+		builder.Services.PostConfigure<GitBranchStateCacheOptions>(options => ReplaceAllowLists(options, allowLists));
 
 		// 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
@@ -260,23 +267,19 @@
 	}
 
 	/// <summary>
-	/// Binds every <c>--allow</c> flag to configuration.
+	/// Groups every <c>--allow</c> flag by the upstream it names.
 	/// </summary>
-	/// <remarks>
-	/// Indexed per upstream so repeating the flag appends rather than overwrites, which is what a
-	/// repeatable option has to do to be useful.
-	/// </remarks>
 	/// <param name="entries">The flag values, or null when the flag was not given.</param>
-	/// <param name="overrides">Configuration to add to.</param>
+	/// <param name="allowLists">The patterns given for each upstream, in the order they were typed.</param>
 	/// <param name="invalid">The first entry that could not be read, when one could not.</param>
 	/// <returns><see langword="true"/> when every entry was well formed.</returns>
-	private static bool TryApplyAllows(
+	internal static bool TryParseAllows(
 		string[]? entries,
-		Dictionary<string, string?> overrides,
+		out Dictionary<string, List<string>> allowLists,
 		out string? invalid)
 	{
 		invalid = null;
-		Dictionary<string, int> counts = new(StringComparer.OrdinalIgnoreCase);
+		allowLists = new(StringComparer.OrdinalIgnoreCase);
 
 		foreach (string entry in entries ?? [])
 		{
@@ -286,12 +289,51 @@
 				return false;
 			}
 
-			int index = counts.TryGetValue(name, out int used) ? used : 0;
-			counts[name] = index + 1;
+			if (!allowLists.TryGetValue(name, out List<string>? patterns))
+			{
+				patterns = [];
+				allowLists[name] = patterns;
+			}
 
-			overrides[$"GitBranchStateCache:Upstreams:{name}:Repositories:{index}"] = pattern;
+			patterns.Add(pattern);
 		}
 
 		return true;
 	}
+
+	/// <summary>
+	/// Makes each upstream named by <c>--allow</c> allow exactly the patterns given for it.
+	/// </summary>
+	/// <remarks>
+	/// Not written as configuration keys like the other flags. Configuration merges arrays by index,
+	/// so <c>Repositories:0</c> from the command line would replace only the first entry of a list from
+	/// a file or the environment and leave the rest in force: a file allowing <c>studio/game.git</c> and
+	/// <c>studio/tools.git</c> plus <c>--allow github=studio/engine.git</c> would bind
+	/// <c>studio/engine.git</c> and <c>studio/tools.git</c>, silently dropping <c>studio/game.git</c>.
+	/// Assigning the list after binding, as a post-configure step, is what makes the flag replace the
+	/// list rather than patch it. Upstreams no flag names keep the list configuration gave them. This
+	/// is the same behaviour as <c>ktsu.GitLfsCache</c>, so the two tools read their flags alike.
+	/// </remarks>
+	/// <param name="options">The options as bound from configuration.</param>
+	/// <param name="allowLists">The patterns given for each upstream.</param>
+	internal static void ReplaceAllowLists(
+		GitBranchStateCacheOptions options,
+		IReadOnlyDictionary<string, List<string>> allowLists)
+	{
+		foreach ((string name, List<string> patterns) in allowLists)
+		{
+			if (!options.Upstreams.TryGetValue(name, out UpstreamOptions? upstream))
+			{
+				upstream = new UpstreamOptions();
+				options.Upstreams[name] = upstream;
+			}
+
+			upstream.Repositories.Clear();
+
+			foreach (string pattern in patterns)
+			{
+				upstream.Repositories.Add(pattern);
+			}
+		}
+	}
 }

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. 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. 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.