diff --git a/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs b/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs index efbfd33..6f6ffd5 100644 --- a/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs +++ b/GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs @@ -152,6 +152,20 @@ public void Validate_NonPositiveMaxCachedMergeBases_Fails() Assert.Contains("MaxCachedMergeBases", result.FailureMessage); } + [TestMethod] + [DataRow(0)] + [DataRow(-1)] + public void Validate_NonPositiveMaxBranchPatternsPerRequest_Fails(int limit) + { + GitBranchStateCacheOptions options = Valid(); + options.MaxBranchPatternsPerRequest = limit; + + ValidateOptionsResult result = Validate(options); + + Assert.IsTrue(result.Failed); + Assert.Contains("MaxBranchPatternsPerRequest", result.FailureMessage); + } + [TestMethod] public void Validate_NonPositiveMaxConcurrentBranchQueries_Fails() { diff --git a/GitBranchStateCache.Tests/Integration/StateFlowTests.cs b/GitBranchStateCache.Tests/Integration/StateFlowTests.cs index 48fe198..14941b6 100644 --- a/GitBranchStateCache.Tests/Integration/StateFlowTests.cs +++ b/GitBranchStateCache.Tests/Integration/StateFlowTests.cs @@ -258,6 +258,66 @@ public async Task Post_State_WithNoBranchPatterns_Is400() Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode); } + [TestMethod] + public async Task Post_State_MoreBranchPatternsThanTheLimit_Is400BeforeAnyFetch() + { + // Every pattern is matched against every branch, synchronously, so the count has to be + // refused before the mirror is cloned or fetched. + await using ServiceFixture fixture = await ServiceFixture.StartAsync(new() + { + ["GitBranchStateCache:MaxBranchPatternsPerRequest"] = "3", + }); + Seed(fixture.Git); + + string[] patterns = ["origin/main", "origin/release/*", "origin/a*", "origin/b*"]; + + using HttpResponseMessage response = await fixture.Client.SendAsync( + StateRequest(JsonSerializer.Serialize(new { @base = ClientBase, branchPatterns = patterns }))); + JsonElement body = await ReadAsync(response); + + Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Contains("at most 3", body.GetProperty("message").GetString()!); + Assert.AreEqual(0, fixture.Git.CountOf("clone")); + Assert.AreEqual(0, fixture.Git.CountOf("fetch")); + } + + [TestMethod] + public async Task Post_State_BranchPatternsAtTheLimit_AreAccepted() + { + await using ServiceFixture fixture = await ServiceFixture.StartAsync(new() + { + ["GitBranchStateCache:MaxBranchPatternsPerRequest"] = "2", + }); + Seed(fixture.Git); + + string[] patterns = ["origin/main", "origin/release/*"]; + + using HttpResponseMessage response = await fixture.Client.SendAsync( + StateRequest(JsonSerializer.Serialize(new { @base = ClientBase, branchPatterns = patterns }))); + + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + } + + [TestMethod] + public async Task Get_Branches_MoreBranchPatternsThanTheLimit_Is400() + { + await using ServiceFixture fixture = await ServiceFixture.StartAsync(new() + { + ["GitBranchStateCache:MaxBranchPatternsPerRequest"] = "1", + }); + Seed(fixture.Git); + + using HttpRequestMessage request = new( + HttpMethod.Get, + "/v1/github/studio/game.git/branches?pattern=origin/main&pattern=origin/release/*"); + request.Headers.Authorization = AuthenticationHeaderValue.Parse(Credential); + + using HttpResponseMessage response = await fixture.Client.SendAsync(request); + + Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode); + Assert.AreEqual(0, fixture.Git.CountOf("clone")); + } + [TestMethod] [DataRow("HEAD")] [DataRow("main@{1}")] diff --git a/GitBranchStateCache.Tool/appsettings.json b/GitBranchStateCache.Tool/appsettings.json index 52703b2..93d2f2b 100644 --- a/GitBranchStateCache.Tool/appsettings.json +++ b/GitBranchStateCache.Tool/appsettings.json @@ -16,6 +16,7 @@ "MaxCachedMergeBases": 20000, "MaxConcurrentBranchQueries": 4, "MaxPathsPerRequest": 20000, + "MaxBranchPatternsPerRequest": 256, "MirrorIdleMaxAge": "30.00:00:00", "MaintenanceInterval": "01:00:00" } diff --git a/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs b/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs index d591166..3f8703a 100644 --- a/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs +++ b/GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs @@ -123,6 +123,15 @@ public sealed class GitBranchStateCacheOptions /// public int MaxPathsPerRequest { get; set; } = 20_000; + /// + /// Gets or sets the most branch patterns one request may name. + /// + /// + /// Every pattern is matched against every branch in the mirror, synchronously, so without a limit + /// one request can keep a thread busy for minutes. A request over it is refused before any fetch. + /// + public int MaxBranchPatternsPerRequest { get; set; } = 256; + /// /// Gets or sets how long a mirror may go unqueried before it is deleted. /// diff --git a/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs b/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs index f304039..b96b012 100644 --- a/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs +++ b/GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs @@ -167,6 +167,11 @@ private static void ValidateLimits(GitBranchStateCacheOptions options, List failures) diff --git a/GitBranchStateCache/Endpoints/BranchStateHandler.cs b/GitBranchStateCache/Endpoints/BranchStateHandler.cs index 54f18e7..1d1656a 100644 --- a/GitBranchStateCache/Endpoints/BranchStateHandler.cs +++ b/GitBranchStateCache/Endpoints/BranchStateHandler.cs @@ -141,7 +141,11 @@ private async Task HandleStateAsync( + "which 'git rev-parse @{upstream}' produces without touching the network."); } - if (!TryParsePatterns(request.BranchPatterns, out List patterns, out string? patternFailure)) + if (!TryParsePatterns( + request.BranchPatterns, + options.Value.MaxBranchPatternsPerRequest, + out List patterns, + out string? patternFailure)) { return BadRequest(patternFailure!); } @@ -407,7 +411,11 @@ .. context.Request.Query["pattern"] .Select(pattern => pattern!) ]; - if (!TryParsePatterns(requested, out List patterns, out string? patternFailure)) + if (!TryParsePatterns( + requested, + options.Value.MaxBranchPatternsPerRequest, + out List patterns, + out string? patternFailure)) { return BadRequest(patternFailure!); } @@ -532,6 +540,7 @@ private static string WithoutGitSuffix(string repositoryPath) private static bool TryParsePatterns( IReadOnlyList? requested, + int limit, out List patterns, out string? failure) { @@ -544,6 +553,12 @@ private static bool TryParsePatterns( return false; } + if (requested.Count > limit) + { + failure = $"The request named {requested.Count} branch patterns, and at most {limit} may be sent."; + return false; + } + foreach (string pattern in requested) { if (!BranchPattern.TryParse(pattern, out BranchPattern? parsed, out string? reason)) diff --git a/README.md b/README.md index b955ad3..d59079f 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,7 @@ Resolves wildcard patterns against the mirror's refs and returns names with tip "MaxCachedMergeBases": 20000, "MaxConcurrentBranchQueries": 4, "MaxPathsPerRequest": 20000, + "MaxBranchPatternsPerRequest": 256, "MirrorIdleMaxAge": "30.00:00:00", "Upstreams": { "github": { @@ -165,6 +166,8 @@ Refs younger than `RefsTtl` are answered from as they are. Past it, and for up t Merge bases are cached by mirror, client base and branch tip, up to `MaxCachedMergeBases`, and diffs by merge base and tip, up to `MaxCachedDiffs`. A request whose answers are all cached starts no git processes beyond reading the refs. One that is not works on up to `MaxConcurrentBranchQueries` branches at a time. Each clone and fetch also writes a commit-graph, which is what keeps an uncached merge base quick on a long history. +A request may name up to `MaxPathsPerRequest` paths and up to `MaxBranchPatternsPerRequest` branch patterns. Every pattern is matched against every branch in the mirror, so a request over either limit is refused with a 400 before anything is fetched. + ## Deployment Deploy **adjacent to the forge, not on-premises**. Its cost is round trips and its clients are worst served on residential links, which is the opposite placement from an object cache. `deploy/k8s` is a kustomize base: a StatefulSet with a read-write-once volume, a service, an ingress and a configmap. Start at one replica; each replica holds its own mirrors and diff cache, so replicas multiply fetch traffic and disk without improving the hit rate.