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

GitLfsCache.Tests/GitLfsCache.Tests.csproj

@@ -26,6 +26,7 @@
 
   <ItemGroup>
     <ProjectReference Include="..\GitLfsCache\GitLfsCache.csproj" />
+    <ProjectReference Include="..\GitLfsCache.Tool\GitLfsCache.Tool.csproj" />
   </ItemGroup>
 
   <ItemGroup>

GitLfsCache.Tests/Tool/AllowFlagTests.cs

@@ -0,0 +1,98 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.GitLfsCache.Tests.Tool;
+
+using System.Text;
+using ktsu.GitLfsCache.Configuration;
+using ktsu.GitLfsCache.Tool;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
+
+[TestClass]
+public class AllowFlagTests
+{
+	private const string ConfigFile = """
+		{
+		  "GitLfsCache": {
+		    "Upstreams": {
+		      "github": { "BaseUrl": "https://github.com", "Repositories": ["studio/**", "**"] },
+		      "ado": { "BaseUrl": "https://dev.azure.com/org", "Repositories": ["project/**", "other/**"] }
+		    }
+		  }
+		}
+		""";
+
+	/// <summary>
+	/// Binds the options the way the tool does: a configuration file, then the flags over it.
+	/// </summary>
+	private static GitLfsCacheOptions 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<GitLfsCacheOptions>().Bind(configuration.GetSection(GitLfsCacheOptions.SectionName));
+		services.PostConfigure<GitLfsCacheOptions>(options => Program.ReplaceAllowLists(options, allowLists));
+
+		using ServiceProvider provider = services.BuildServiceProvider();
+		return provider.GetRequiredService<IOptions<GitLfsCacheOptions>>().Value;
+	}
+
+	private static void AssertRepositories(GitLfsCacheOptions options, string upstream, params string[] expected) =>
+		CollectionAssert.AreEqual(expected, options.Upstreams[upstream].Repositories.ToArray());
+
+	[TestMethod]
+	public void Allow_FewerPatternsThanConfigured_ReplacesTheConfiguredList()
+	{
+		GitLfsCacheOptions options = Bind("github=only/**");
+
+		AssertRepositories(options, "github", "only/**");
+	}
+
+	[TestMethod]
+	public void Allow_Repeated_KeepsEveryPatternInOrder()
+	{
+		GitLfsCacheOptions options = Bind("github=a/**", "GitHub=b/**");
+
+		AssertRepositories(options, "github", "a/**", "b/**");
+	}
+
+	[TestMethod]
+	public void Allow_ForOneUpstream_LeavesTheOthersConfiguredList()
+	{
+		GitLfsCacheOptions options = Bind("github=only/**");
+
+		AssertRepositories(options, "ado", "project/**", "other/**");
+	}
+
+	[TestMethod]
+	public void Allow_ForAnUnconfiguredUpstream_AddsItWithThoseRepositories()
+	{
+		GitLfsCacheOptions options = Bind("gitlab=team/**");
+
+		AssertRepositories(options, "gitlab", "team/**");
+	}
+
+	[TestMethod]
+	public void NoAllow_KeepsTheConfiguredList()
+	{
+		GitLfsCacheOptions options = Bind();
+
+		AssertRepositories(options, "github", "studio/**", "**");
+	}
+
+	[TestMethod]
+	[DataRow("github")]
+	[DataRow("=studio/**")]
+	[DataRow("github=")]
+	public void TryParseAllows_Malformed_ReportsTheEntry(string entry)
+	{
+		Assert.IsFalse(Program.TryParseAllows([entry], out _, out string? invalid));
+		Assert.AreEqual(entry, invalid);
+	}
+}

GitLfsCache.Tool/Program.cs

@@ -5,6 +5,7 @@
 using System.CommandLine;
 using System.Security.Cryptography;
 using ktsu.Essentials;
+using ktsu.GitLfsCache.Configuration;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.HttpOverrides;
 using Microsoft.Extensions.Configuration;
@@ -28,7 +29,7 @@
 	/// <summary>The configuration key the <c>--token-key</c> flag overrides.</summary>
 	private const string TokenKeyKey = "GitLfsCache:TokenKeys:0";
 
 	private static async Task<int> Main(string[] args)
 	{
 		Option<int?> port = new("--port", "-p")
 		{
@@ -134,14 +135,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/**.")
 					.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)
@@ -151,6 +156,7 @@
 
 	private static async Task<int> RunAsync(
 		Dictionary<string, string?> overrides,
+		Dictionary<string, List<string>> allowLists,
 		int port,
 		string? configPath,
 		CancellationToken cancellationToken)
@@ -173,9 +179,10 @@
 
 		ApplyDefaults(builder.Configuration, overrides);
 		builder.Configuration.AddInMemoryCollection(overrides);
 		builder.Configuration["Kestrel:Endpoints:Http:Url"] = $"http://*:{port}";
 
 		builder.Services.AddGitLfsCache(builder.Configuration);
+		builder.Services.PostConfigure<GitLfsCacheOptions>(options => ReplaceAllowLists(options, allowLists));
 
 		// Behind an ingress the request the proxy sees is not the URL the client used, so the scheme
 		// and host from the forwarded headers are what make derived transfer URLs correct. Without
@@ -279,23 +286,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 ?? [])
 		{
@@ -305,15 +308,52 @@
 				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[$"GitLfsCache: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/**</c> and
+	/// <c>**</c> plus <c>--allow github=only/**</c> would still allow everything. Assigning the list
+	/// after binding, as a post-configure step, is what makes the flag narrow the list rather than patch
+	/// it. Upstreams no flag names keep the list configuration gave them.
+	/// </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(
+		GitLfsCacheOptions 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);
+			}
+		}
+	}
+
 	private static void ApplyDefaults(ConfigurationManager configuration, Dictionary<string, string?> overrides)
 	{
 		bool hasStore = overrides.ContainsKey("GitLfsCache:Store:Root")

README.md

@@ -95,7 +95,7 @@ Nothing has to be passed as a flag. Configuration comes from three places, each
 2. **A file named with `--config`**, which can live anywhere: `gitlfscache --config /etc/gitlfscache.json`. It is layered over the working-directory file rather than replacing it, so an explicit file only has to carry what differs. A path that does not exist is reported by name and the process exits rather than starting on defaults.
 3. **Environment variables**, using `__` as the section separator (`GitLfsCache__Store__MaxSize`, `GitLfsCache__Upstreams__github__BaseUrl`). This is how the Kubernetes base configures everything.
 
-The flags are a convenience over the same settings and win over all three, so `--max-size 3GB` beats a `--config` file asking for 9GB.
+The flags are a convenience over the same settings and win over all three, so `--max-size 3GB` beats a `--config` file asking for 9GB. `--allow` replaces rather than adds to: when it names an upstream, that upstream allows exactly the patterns given on the command line, and any `Repositories` entries for it from the three sources above are discarded. Upstreams no `--allow` names keep their configured list.
 
 ```json
 {