From c78363ea775c8229deae3e3932a252f519035117 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Tue, 25 Aug 2026 17:04:25 +0200 Subject: [PATCH 1/4] =?UTF-8?q?feat(changelog):=20add=20link:=20field=20to?= =?UTF-8?q?=20changelog=20entries=20=E2=80=94=20marker=20read=20side?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `link:` to `ChangelogEntryDto`, `ChangelogEntry` (domain type), `BundledEntry`, and `BundledEntryDto` so the field survives the full YAML → domain → YAML round-trip and the scrub pass. `ChangelogEntry.IsMarker` returns true when `Link` is non-null. A marker is a machine-written object containing only `link: {prNumber}` that redirects a non-primary PR to the parent entry's key; no markers exist in S3 today (the writer lands in Step 8), so this is inert on merge. `ChangelogContentScrubber.ScrubChangelog` explicitly preserves `link:` in both the `BundledEntry` passed to `LinkAllowlistSanitizer` and the `with` expression applied after sanitization. `link:` is never a URL, so no allowlist rule touches it. Tests: serialization round-trip for marker entries; scrub preserves `link:` when prs: are stripped; `IsMarker` false on normal entries. Co-Authored-By: Claude Sonnet 4.6 --- .../ReleaseNotes/Bundle.cs | 1 + .../ReleaseNotes/ChangelogEntry.cs | 6 ++ .../ReleaseNotes/ReleaseNotesSerialization.cs | 15 +++-- .../ReleaseNotes/BundledEntry.cs | 6 ++ .../ReleaseNotes/ChangelogEntry.cs | 13 +++- .../Scrubbing/ChangelogContentScrubber.cs | 6 +- .../ChangelogContentScrubberTests.cs | 67 +++++++++++++++++++ .../ReleaseNotesSerializationTests.cs | 38 +++++++++++ 8 files changed, 144 insertions(+), 8 deletions(-) create mode 100644 tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs index 7cf3977e62..cbfe9b0116 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs @@ -83,6 +83,7 @@ public sealed record BundledEntryDto public string? Pr { get; set; } public List? Prs { get; set; } public List? Issues { get; set; } + public string? Link { get; set; } } /// diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs index 33b7bd3b4b..1949fb2330 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogEntry.cs @@ -27,6 +27,12 @@ public record ChangelogEntryDto [YamlMember(Alias = "feature-id", ApplyNamingConventions = false)] public string? FeatureId { get; set; } public bool? Highlight { get; set; } + /// + /// Marker reference: a bare PR number pointing to the authoritative entry in the same pool. + /// A marker carries link: and nothing else; any other field alongside it is invalid. + /// Written by the pipeline for non-primary PRs in a multi-PR entry; never hand-authored. + /// + public string? Link { get; set; } } /// diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs index a322ebb272..bcdde16809 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs @@ -165,7 +165,8 @@ private static string ToYamlDoubleQuotedString(string s) Impact = dto.Impact, Action = dto.Action, FeatureId = dto.FeatureId, - Highlight = dto.Highlight + Highlight = dto.Highlight, + Link = dto.Link }; private static ChangelogEntry ToEntry(BundledEntry entry) => new() @@ -181,7 +182,8 @@ private static string ToYamlDoubleQuotedString(string s) Impact = entry.Impact, Action = entry.Action, FeatureId = entry.FeatureId, - Highlight = entry.Highlight + Highlight = entry.Highlight, + Link = entry.Link }; private static ProductReference ToProductReference(ProductInfoDto dto) => new() @@ -225,7 +227,8 @@ private static string ToYamlDoubleQuotedString(string s) Subtype = ParseEntrySubtype(dto.Subtype), Areas = dto.Areas, Prs = dto.Prs ?? (dto.Pr != null ? [dto.Pr] : null), - Issues = dto.Issues + Issues = dto.Issues, + Link = dto.Link }; private static BundledFile ToBundledFile(BundledFileDto dto) => new() @@ -294,7 +297,8 @@ private static ChangelogEntryType ParseEntryType(string? value) Impact = entry.Impact, Action = entry.Action, FeatureId = entry.FeatureId, - Highlight = entry.Highlight + Highlight = entry.Highlight, + Link = entry.Link }; private static ProductInfoDto ToDto(ProductReference product) => new() @@ -338,7 +342,8 @@ private static ChangelogEntryType ParseEntryType(string? value) Subtype = EntrySubtypeToString(entry.Subtype), Areas = entry.Areas?.ToList(), Prs = entry.Prs?.ToList(), - Issues = entry.Issues?.ToList() + Issues = entry.Issues?.ToList(), + Link = entry.Link }; private static BundledFileDto ToDto(BundledFile file) => new() diff --git a/src/Elastic.Documentation/ReleaseNotes/BundledEntry.cs b/src/Elastic.Documentation/ReleaseNotes/BundledEntry.cs index e33c2cf833..ffce23ff10 100644 --- a/src/Elastic.Documentation/ReleaseNotes/BundledEntry.cs +++ b/src/Elastic.Documentation/ReleaseNotes/BundledEntry.cs @@ -47,4 +47,10 @@ public record BundledEntry /// Related issue URLs or references. public IReadOnlyList? Issues { get; init; } + + /// + /// Marker reference: a bare PR number pointing to the authoritative entry in the same pool. + /// Preserved through the scrub round-trip so the public pool can resolve markers end-to-end. + /// + public string? Link { get; init; } } diff --git a/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs b/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs index 28f8091ad7..25062776c6 100644 --- a/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs +++ b/src/Elastic.Documentation/ReleaseNotes/ChangelogEntry.cs @@ -46,6 +46,16 @@ public record ChangelogEntry /// Whether this entry should be highlighted. public bool? Highlight { get; init; } + /// + /// Marker reference: a bare PR number pointing to the authoritative entry in the same pool. + /// Non-null only on machine-written marker objects emitted by the pipeline for non-primary PRs + /// in a multi-PR entry. A marker carries link: and nothing else. + /// + public string? Link { get; init; } + + /// True when this entry is a pipeline-written marker that redirects to another PR's entry. + public bool IsMarker => Link is not null; + /// /// Converts this ChangelogEntry to a BundledEntry for embedding in bundles. /// File property is set to null; set it separately using a 'with' expression. @@ -64,6 +74,7 @@ public record ChangelogEntry Subtype = Subtype, Areas = Areas, Prs = Prs, - Issues = Issues + Issues = Issues, + Link = Link }; } diff --git a/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs b/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs index 9fd627e515..fc479522ad 100644 --- a/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs +++ b/src/services/Elastic.Changelog/Scrubbing/ChangelogContentScrubber.cs @@ -85,7 +85,8 @@ private async Task ScrubChangelog(string content, Cancel ctx) Issues = entry.Issues, Areas = entry.Areas, Highlight = entry.Highlight, - Subtype = entry.Subtype + Subtype = entry.Subtype, + Link = entry.Link }; await using var collector = new DiagnosticsCollector([]); @@ -107,7 +108,8 @@ private async Task ScrubChangelog(string content, Cancel ctx) Impact = sanitized.Impact, Action = sanitized.Action, Prs = sanitized.Prs, - Issues = sanitized.Issues + Issues = sanitized.Issues, + Link = entry.Link }; var result = ReleaseNotesSerialization.SerializeEntry(scrubEntry); diff --git a/tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs b/tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs new file mode 100644 index 0000000000..4a341c0a45 --- /dev/null +++ b/tests/Elastic.Changelog.Tests/Scrubbing/ChangelogContentScrubberTests.cs @@ -0,0 +1,67 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using AwesomeAssertions; +using Elastic.Changelog.Scrubbing; +using Elastic.Documentation.Configuration.ReleaseNotes; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Elastic.Changelog.Tests.Scrubbing; + +public class ChangelogContentScrubberTests +{ + private static readonly IReadOnlyList AllowAll = ["elastic/elasticsearch"]; + + private static ChangelogContentScrubber Scrubber(IReadOnlyList? allowRepos = null) => + new(NullLoggerFactory.Instance, allowRepos ?? AllowAll); + + [Fact] + public async Task ScrubAsync_MarkerEntry_LinkFieldPreserved() + { + // A marker is link: only — the scrubber must pass it through unchanged. + var yaml = "link: \"12345\"\n"; + var scrubber = Scrubber(); + + var result = await scrubber.ScrubAsync("changelog/elastic/elasticsearch/main/12346.yaml", yaml, CancellationToken.None); + + var entry = ReleaseNotesSerialization.DeserializeEntry(result); + entry.Link.Should().Be("12345", "link: must survive the scrub round-trip"); + entry.IsMarker.Should().BeTrue(); + } + + [Fact] + public async Task ScrubAsync_EntryWithLinkAndPrivatePrs_LinkPreservedPrsStripped() + { + // A hypothetical entry with both link: and prs: pointing to a private repo. + // link: must survive scrubbing; private prs: must be stripped. + var yaml = + "link: \"12345\"\n" + + "prs:\n" + + " - https://github.com/elastic/private-repo/pull/99\n"; + var scrubber = Scrubber(["elastic/elasticsearch"]); // private-repo not allowed + + var result = await scrubber.ScrubAsync("changelog/elastic/elasticsearch/main/12346.yaml", yaml, CancellationToken.None); + + var entry = ReleaseNotesSerialization.DeserializeEntry(result); + entry.Link.Should().Be("12345", "link: must survive the scrub even when prs: is stripped"); + entry.Prs.Should().BeNullOrEmpty("private PR reference must be scrubbed"); + } + + [Fact] + public async Task ScrubAsync_NormalEntry_LinkIsNull() + { + var yaml = + "title: Fix search performance\n" + + "type: bug-fix\n" + + "products:\n" + + " - product: elasticsearch\n"; + var scrubber = Scrubber(); + + var result = await scrubber.ScrubAsync("changelog/elastic/elasticsearch/main/12345.yaml", yaml, CancellationToken.None); + + var entry = ReleaseNotesSerialization.DeserializeEntry(result); + entry.Link.Should().BeNull(); + entry.IsMarker.Should().BeFalse(); + } +} diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs index 91f038a0db..2a1a3352fc 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs @@ -150,4 +150,42 @@ public void SerializeEntry_InjectedFieldInTitle_DoesNotPolluteOtherFields() roundTrip.Impact.Should().BeNull(); roundTrip.Action.Should().BeNull(); } + + [Fact] + public void SerializeDeserialize_MarkerEntry_LinkRoundTrips() + { + // A marker is link: only — no title, type, products. + var yaml = "link: \"12345\"\n"; + + var entry = ReleaseNotesSerialization.DeserializeEntry(yaml); + + entry.Link.Should().Be("12345"); + entry.IsMarker.Should().BeTrue(); + entry.Title.Should().Be("", "title is empty when absent"); + entry.Type.Should().Be(ChangelogEntryType.Invalid); + } + + [Fact] + public void SerializeEntry_WithLink_LinkRoundTrips() + { + var entry = new ChangelogEntry { Link = "99999" }; + + var yaml = ReleaseNotesSerialization.SerializeEntry(entry); + var roundTrip = ReleaseNotesSerialization.DeserializeEntry(yaml); + + roundTrip.Link.Should().Be("99999"); + roundTrip.IsMarker.Should().BeTrue(); + } + + [Fact] + public void IsMarker_NullLink_ReturnsFalse() + { + var entry = new ChangelogEntry + { + Title = "A real entry", + Type = ChangelogEntryType.Feature + }; + + entry.IsMarker.Should().BeFalse(); + } } From a565180c786ce659d0f53c9ca12c68de0079c7cc Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Tue, 25 Aug 2026 19:04:01 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(changelog):=20marker=20entries=20serial?= =?UTF-8?q?ize=20as=20link-only=20=E2=80=94=20omit=20title=20and=20other?= =?UTF-8?q?=20non-link=20fields?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ToDto(ChangelogEntry) now short-circuits on IsMarker: only Link is emitted. This enforces the marker contract (link: is the sole field) and prevents 'title: ''' from appearing in round-tripped marker YAML. New test: SerializeEntry_MarkerEntry_YamlContainsOnlyLinkField. Co-Authored-By: Claude Sonnet 4.6 --- .../ReleaseNotes/ReleaseNotesSerialization.cs | 37 +++++++++++-------- .../ReleaseNotesSerializationTests.cs | 14 +++++++ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs index bcdde16809..236a9cafbd 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs @@ -284,22 +284,29 @@ private static ChangelogEntryType ParseEntryType(string? value) // Reverse mappings (Domain → DTO) for serialization - private static ChangelogEntryDto ToDto(ChangelogEntry entry) => new() + private static ChangelogEntryDto ToDto(ChangelogEntry entry) { - Prs = entry.Prs?.ToList(), - Issues = entry.Issues?.ToList(), - Type = EntryTypeToString(entry.Type), - Subtype = EntrySubtypeToString(entry.Subtype), - Products = entry.Products?.Select(ToDto).ToList(), - Areas = entry.Areas?.ToList(), - Title = entry.Title, - Description = entry.Description, - Impact = entry.Impact, - Action = entry.Action, - FeatureId = entry.FeatureId, - Highlight = entry.Highlight, - Link = entry.Link - }; + // Marker entries are link-only; emitting any other field would violate the marker contract. + if (entry.IsMarker) + return new ChangelogEntryDto { Link = entry.Link }; + + return new ChangelogEntryDto + { + Prs = entry.Prs?.ToList(), + Issues = entry.Issues?.ToList(), + Type = EntryTypeToString(entry.Type), + Subtype = EntrySubtypeToString(entry.Subtype), + Products = entry.Products?.Select(ToDto).ToList(), + Areas = entry.Areas?.ToList(), + Title = entry.Title, + Description = entry.Description, + Impact = entry.Impact, + Action = entry.Action, + FeatureId = entry.FeatureId, + Highlight = entry.Highlight, + Link = entry.Link + }; + } private static ProductInfoDto ToDto(ProductReference product) => new() { diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs index 2a1a3352fc..f8a88914c1 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/ReleaseNotesSerializationTests.cs @@ -177,6 +177,20 @@ public void SerializeEntry_WithLink_LinkRoundTrips() roundTrip.IsMarker.Should().BeTrue(); } + [Fact] + public void SerializeEntry_MarkerEntry_YamlContainsOnlyLinkField() + { + // Marker entries must serialize as link: only — no title, type, products, etc. + var entry = new ChangelogEntry { Link = "12345" }; + + var yaml = ReleaseNotesSerialization.SerializeEntry(entry); + + yaml.Should().Contain("link:"); + yaml.Should().NotContain("title:"); + yaml.Should().NotContain("type:"); + yaml.Should().NotContain("products:"); + } + [Fact] public void IsMarker_NullLink_ReturnsFalse() { From 66f8e8e97e2ba3d95b2185ac8023f7395c82afab Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Tue, 25 Aug 2026 18:48:05 +0200 Subject: [PATCH 3/4] fix(changelog): probe-based CDN sourcing replaces dead pool-registry read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace FetchAsync (reads changelog/{org}/{repo}/{branch}/registry.json, which nothing has written since #3760) with per-PR probing for the --prs / --report / --release-version CDN path. For each requested PR, the bundler now fetches {pr}.yaml directly: - 404 = no changelog for this PR (warn + skip, not retried — it is authoritative) - 5xx / transport errors use the existing retry budget - Marker entries (link: {parentPr}) are resolved depth-1; marker chains and missing parents are hard errors - Duplicate markers pointing to the same parent yield one entry Also adds an explicit --issues CDN error (issue→PR resolution requires reading every body, which probing cannot do). Fixes the reported regression: changelog bundle --prs against CDN returned zero entries because the pool registry was never written after #3760 retired the writer. Co-Authored-By: Claude Sonnet 4.6 --- .../ReleaseNotes/CdnChangelogEntryFetcher.cs | 48 ++++ .../Bundling/ChangelogBundlingService.cs | 135 +++++++++- .../Changelogs/BundleCdnSourcingTests.cs | 247 ++++++++++++++---- 3 files changed, 369 insertions(+), 61 deletions(-) diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs index 9e69933f21..329bf936dd 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs @@ -291,6 +291,54 @@ public async Task> FetchAsync( return (false, await response.Content.ReadAsStringAsync(ctx).ConfigureAwait(false)); } + /// + /// Probes for a PR's changelog entry by number. Returns null when the entry does not exist (404); + /// this is the normal "no changelog for this PR" case and is not an error. Transient failures + /// (5xx, timeouts, transport errors) are retried with the same budget as other fetches. + /// Unlike , a 404 here is authoritative-null, not an error. + /// + public async Task FetchPrEntryAsync( + Uri baseUri, string org, string repo, string branch, int prNumber, Cancel ctx) + { + var fileName = $"{prNumber}.yaml"; + var poolSegments = ChangelogKeys.PoolSegments(org, repo, branch); + var uri = CombineSegments(baseUri, [.. poolSegments, fileName]); + var poolLabel = $"{org}/{repo}/{branch}"; + var (fetched, content, _) = await TryProbeEntryAsync(uri, fileName, poolLabel, ctx).ConfigureAwait(false); + return fetched ? new CdnChangelogEntry(fileName, content) : null; + } + + private async Task<(bool Fetched, string Content, string? LastError)> TryProbeEntryAsync(Uri uri, string fileName, string poolLabel, Cancel ctx) + { + string? lastError = null; + for (var attempt = 1; attempt <= _maxAttempts; attempt++) + { + ctx.ThrowIfCancellationRequested(); + try + { + var (notFound, content) = await FetchTextOrNotFoundAsync(uri, attempt, ctx).ConfigureAwait(false); + if (notFound) + return (false, string.Empty, null); // 404 = probe miss, not an error + if (attempt > 1) + _logger.LogInformation("Probed changelog entry '{File}' for {Pool} on attempt {Attempt}/{Max}", fileName, poolLabel, attempt, _maxAttempts); + return (true, content, null); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + if (ex is HttpRequestException { StatusCode: >= HttpStatusCode.BadRequest and < HttpStatusCode.InternalServerError }) + return (false, string.Empty, ex.Message); + lastError = ex.Message; + if (attempt >= _maxAttempts) + break; + var delay = RetryDelay(attempt); + _logger.LogDebug("Probe for changelog entry '{File}' for {Pool} failed (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", + fileName, poolLabel, attempt, _maxAttempts, ex.Message, delay); + await _sleep(delay, ctx).ConfigureAwait(false); + } + } + return (false, string.Empty, lastError); + } + /// /// Fetches a single entry, retrying transient failures (most importantly a not-yet-propagated 404) /// up to times with exponential backoff. Retry requests are cache-busted diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index 3e9dcea447..b9f0aecbce 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -289,11 +289,13 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle if (!ValidateInput(collector, input, requireDirectoryExists: !useCdn)) return false; - // --all and --input-products require reading every entry body; that is only possible locally. - // On the CDN path entries are probed by key, so there is nothing to enumerate without a PR list. - if (useCdn && (input.All || input.InputProducts is { Count: > 0 })) + // --all, --input-products, and --issues require reading every entry body; that is only possible locally. + // On the CDN path entries are probed by key (one per PR), so there is nothing to enumerate without a PR list. + if (useCdn && (input.All || input.InputProducts is { Count: > 0 } || input.Issues is { Length: > 0 })) { - var flag = input.All ? "--all" : "--input-products"; + var flag = input.All ? "--all" + : input.Issues is { Length: > 0 } ? "--issues" + : "--input-products"; collector.EmitError(string.Empty, $"{flag} is not supported when sourcing changelog entries from the CDN, because entries are fetched by key (one per PR) and there is no pool enumeration. " + "Pass --force-local or --directory to bundle from a local checkout instead."); @@ -380,12 +382,16 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle } else { - // --prs / --issues on the CDN path: still uses the pool registry for now (Step 9 will - // switch this to per-PR probing once canonical keys and markers are in place). - var contents = await FetchCdnEntriesAsync(collector, authoringOwner, authoringRepo, authoringBranch, ctx); - if (contents == null) + // --prs / --report / --release-version on the CDN path: probe one key per PR number. + // Each entry lives at changelog/{org}/{repo}/{branch}/{pr}.yaml; 404 = no entry for that PR. + var probed = await FetchCdnProbedEntriesAsync(collector, authoringOwner, authoringRepo, authoringBranch, prsToMatch, ctx); + if (probed == null) return false; - matchResult = entryMatcher.MatchChangelogContents(collector, contents, filterCriteria, ctx); + _logger.LogInformation("Probed {Count} changelog entry(ies) for {Pool} from CDN", + probed.Count, $"{authoringOwner}/{authoringRepo}/{authoringBranch}"); + // Entries are already selected by probe; pass IncludeAll so the matcher skips content re-filtering. + var probeCriteria = filterCriteria with { IncludeAll = true }; + matchResult = entryMatcher.MatchChangelogContents(collector, probed, probeCriteria, ctx); } } else @@ -1324,6 +1330,117 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return combined; } + private async Task?> FetchCdnProbedEntriesAsync( + IDiagnosticsCollector collector, + string? org, + string? repo, + string? branch, + HashSet prsToMatch, + Cancel ctx) + { + if (string.IsNullOrWhiteSpace(repo)) + { + collector.EmitError(string.Empty, + "Sourcing changelog entries from the CDN requires a resolvable authoring repository. " + + "Set bundle.repo in changelog.yml (or pass --repo), or pass --force-local / --directory to bundle local changelog files."); + return null; + } + + var resolvedOrg = string.IsNullOrWhiteSpace(org) ? DefaultOwner : org; + var resolvedBranch = string.IsNullOrWhiteSpace(branch) ? DefaultBranch : branch; + var poolLabel = $"{resolvedOrg}/{repo}/{resolvedBranch}"; + + var baseUri = ChangelogCdn.ResolveBaseUri(); + if (baseUri is null) + { + collector.EmitError(string.Empty, + $"No valid changelog CDN base URL is configured. Set the {ChangelogCdn.BaseUrlEnvironmentVariable} environment variable to an absolute http(s) URL."); + return null; + } + + var prNumbers = new List(); + foreach (var pr in prsToMatch) + { + if (TryExtractPrNumber(pr, resolvedOrg, repo, out var prNumber)) + prNumbers.Add(prNumber); + } + + if (prNumbers.Count == 0) + { + _logger.LogInformation("No PR numbers for {Pool} found in filter; returning empty probe result", poolLabel); + return []; + } + + var tasks = prNumbers.Select(pr => + _entryFetcher.FetchPrEntryAsync(baseUri, resolvedOrg, repo, resolvedBranch, pr, ctx)).ToArray(); + var results = await Task.WhenAll(tasks).ConfigureAwait(false); + + var entries = new Dictionary(StringComparer.OrdinalIgnoreCase); + var hasError = false; + + for (var i = 0; i < prNumbers.Count; i++) + { + var prNumber = prNumbers[i]; + var cdnEntry = results[i]; + + if (cdnEntry is null) + { + _logger.LogWarning("No changelog entry found for PR {PrNumber} in {Pool}", prNumber, poolLabel); + continue; + } + + var entry = ReleaseNotesSerialization.DeserializeEntry(cdnEntry.Value.Content); + if (!entry.IsMarker) + { + _ = entries.TryAdd(cdnEntry.Value.FileName, cdnEntry.Value.Content); + continue; + } + + if (!int.TryParse(entry.Link, out var parentPr) || parentPr <= 0) + { + collector.EmitError(string.Empty, + $"Changelog entry '{cdnEntry.Value.FileName}' contains an invalid link: '{entry.Link}'. Expected a positive PR number."); + hasError = true; + continue; + } + + var parentCdnEntry = await _entryFetcher.FetchPrEntryAsync(baseUri, resolvedOrg, repo, resolvedBranch, parentPr, ctx).ConfigureAwait(false); + if (parentCdnEntry is null) + { + collector.EmitError(string.Empty, + $"Changelog entry '{cdnEntry.Value.FileName}' is a marker pointing to PR {parentPr}, but that entry does not exist in {poolLabel}."); + hasError = true; + continue; + } + + var parentEntry = ReleaseNotesSerialization.DeserializeEntry(parentCdnEntry.Value.Content); + if (parentEntry.IsMarker) + { + collector.EmitError(string.Empty, + $"Marker chain detected: '{cdnEntry.Value.FileName}' → '{parentCdnEntry.Value.FileName}' is also a marker. Marker chains are not allowed."); + hasError = true; + continue; + } + + _ = entries.TryAdd(parentCdnEntry.Value.FileName, parentCdnEntry.Value.Content); + } + + return hasError ? null : entries.Select(kv => (kv.Key, kv.Value)).ToList(); + } + + private static bool TryExtractPrNumber(string pr, string authoringOwner, string authoringRepo, out int prNumber) + { + prNumber = 0; + // Normalize to {owner}/{repo}#{number} form; the org/repo may differ from the CDN pool + // (e.g. --owner override renames the pool but the PR URLs still carry the GitHub org). + // We only need the number to build the probe key. + var normalized = NormalizePrForComparison(pr, authoringOwner, authoringRepo); + var hashIndex = normalized.LastIndexOf('#'); + if (hashIndex < 0) + return false; + return int.TryParse(normalized[(hashIndex + 1)..], out prNumber) && prNumber > 0; + } + /// Gate for repo-scoped CDN entry sourcing: true when the authoring repo resolves, local sourcing is not forced (bundle.use_local_changelogs/--force-local/--directory), and a CDN base is configured. private static bool ShouldSourceFromCdn(string? authoringRepo, bool useLocalChangelogs, bool explicitDirectory) { diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs index 1c7b77633b..cdac69fbed 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs @@ -16,6 +16,7 @@ namespace Elastic.Changelog.Tests.Changelogs; /// /// Tests for the changelog bundle command sourcing its individual changelog entries from the /// public CDN (the default when no --directory is passed and bundle.use_local_changelogs is false). +/// Probe-based: one GET per PR number keyed as {pr}.yaml; no pool registry consulted. /// public class BundleCdnSourcingTests(ITestOutputHelper output) : ChangelogTestBase(output) { @@ -43,18 +44,15 @@ public class BundleCdnSourcingTests(ITestOutputHelper output) : ChangelogTestBas - https://github.com/elastic/elasticsearch/pull/999 """; - // language=json - private const string RegistryJson = - """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "1-alpha.yaml" }, { "file": "2-bravo.yaml" } ] }"""; + // language=yaml + private static string MarkerFor(int parentPr) => $"link: \"{parentPr}\"\n"; - private static StubHandler RegistryHandler() => new(req => + private static StubHandler ProbeHandler() => new(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/registry.json", StringComparison.Ordinal)) - return Json(RegistryJson); - if (path.EndsWith("1-alpha.yaml", StringComparison.Ordinal)) + if (path.EndsWith("/100.yaml", StringComparison.Ordinal)) return Yaml(EntryAlpha); - if (path.EndsWith("2-bravo.yaml", StringComparison.Ordinal)) + if (path.EndsWith("/999.yaml", StringComparison.Ordinal)) return Yaml(EntryBravo); return new HttpResponseMessage(HttpStatusCode.NotFound); }); @@ -63,17 +61,16 @@ public class BundleCdnSourcingTests(ITestOutputHelper output) : ChangelogTestBas private static CdnChangelogEntryFetcher Fetcher(ITestOutputHelper output, StubHandler handler) => new(new TestLoggerFactory(output), handler, sleep: (_, _) => Task.CompletedTask); - private CdnChangelogEntryFetcher Fetcher() => Fetcher(Output, RegistryHandler()); + private CdnChangelogEntryFetcher Fetcher() => Fetcher(Output, ProbeHandler()); private string OutputPath() => FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "bundle.yaml"); [Fact] - public async Task OptionMode_RepoResolvable_SourcesAllEntriesFromRepoPoolOnCdn() + public async Task OptionMode_RepoResolvable_ProbesEntriesByPrNumber() { - // Under the artifact-root layout the CDN entry pool is keyed by the authoring repo, not the - // target product. A resolvable repo (here via --repo) is what enables CDN sourcing. - var handler = RegistryHandler(); + // Probe-based: each PR URL is probed as {pr}.yaml directly; no registry.json is read. + var handler = ProbeHandler(); var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, Fetcher(Output, handler)); var output = OutputPath(); @@ -89,26 +86,26 @@ public async Task OptionMode_RepoResolvable_SourcesAllEntriesFromRepoPoolOnCdn() result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); Collector.Errors.Should().Be(0); - // Entries are sourced from the authoring pool, with org/branch defaulting: changelog/{org}/{repo}/{branch}/... - handler.RequestedPaths.Should().Contain("/changelog/elastic/elasticsearch/main/registry.json"); + // Entries are probed by PR number from the authoring pool. + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/100.yaml", StringComparison.Ordinal)); + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/999.yaml", StringComparison.Ordinal)); + handler.RequestedPaths.Should().NotContain(p => p.Contains("registry.json", StringComparison.Ordinal)); var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); bundle.Should().Contain("Alpha"); bundle.Should().Contain("Bravo"); - bundle.Should().Contain("name: 1-alpha.yaml"); } [Fact] - public async Task OptionMode_OwnerAndBranchOverride_SourcesFromThatPoolOnCdn() + public async Task OptionMode_OwnerAndBranchOverride_ProbesFromThatPool() { - // Explicit owner/branch select a specific pool; the branch is stored verbatim (dots kept). - var handler = RegistryHandler(); + var handler = ProbeHandler(); var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, Fetcher(Output, handler)); var output = OutputPath(); var input = new BundleChangelogsArguments { - Prs = ["https://github.com/elastic/elasticsearch/pull/100", "https://github.com/elastic/elasticsearch/pull/999"], + Prs = ["https://github.com/elastic/elasticsearch/pull/100"], Output = output, Owner = "acme-corp", Repo = "elasticsearch", @@ -119,21 +116,21 @@ public async Task OptionMode_OwnerAndBranchOverride_SourcesFromThatPoolOnCdn() result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); Collector.Errors.Should().Be(0); - handler.RequestedPaths.Should().Contain("/changelog/acme-corp/elasticsearch/8.x/registry.json"); + handler.RequestedPaths.Should().Contain(p => + p.Contains("/acme-corp/elasticsearch/8.x/", StringComparison.Ordinal) && + p.EndsWith("/100.yaml", StringComparison.Ordinal)); } [Fact] - public async Task OptionMode_OwnerFromCombinedRepo_SourcesFromThatPool() + public async Task OptionMode_OwnerFromCombinedRepo_ProbesFromThatPool() { - // When --repo is given in owner/repo form and no explicit owner is set, the owner segment must be - // taken from the repo prefix (not defaulted to elastic), so the CDN pool path stays correct. - var handler = RegistryHandler(); + var handler = ProbeHandler(); var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, Fetcher(Output, handler)); var output = OutputPath(); var input = new BundleChangelogsArguments { - Prs = ["https://github.com/elastic/elasticsearch/pull/100", "https://github.com/elastic/elasticsearch/pull/999"], + Prs = ["https://github.com/elastic/elasticsearch/pull/100"], Output = output, Repo = "acme-corp/widget" }; @@ -142,15 +139,13 @@ public async Task OptionMode_OwnerFromCombinedRepo_SourcesFromThatPool() result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); Collector.Errors.Should().Be(0); - handler.RequestedPaths.Should().Contain("/changelog/acme-corp/widget/main/registry.json"); + handler.RequestedPaths.Should().Contain(p => + p.Contains("/acme-corp/widget/main/", StringComparison.Ordinal)); } [Fact] public async Task OptionMode_NoResolvableRepo_FallsBackToLocal() { - // With no --repo, no bundle.repo in config, and no git-remote resolution at the service layer, the - // authoring repo is unresolvable. With no --directory and no use_local_changelogs, the bundler - // still falls back to local folder sourcing rather than hitting the CDN. var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); FileSystem.Directory.CreateDirectory(localDir); await FileSystem.File.WriteAllTextAsync( @@ -165,7 +160,7 @@ await FileSystem.File.WriteAllTextAsync( FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); - var handler = RegistryHandler(); + var handler = ProbeHandler(); var service = new ChangelogBundlingService(LoggerFactory, FileSystem, ConfigurationContext, null, Fetcher(Output, handler)); var output = OutputPath(); @@ -183,8 +178,6 @@ await FileSystem.File.WriteAllTextAsync( [Fact] public async Task OptionMode_UseLocalChangelogs_ForcesLocalEvenWithResolvableRepo() { - // use_local_changelogs is the explicit opt-out: even when the authoring repo resolves (bundle.repo), - // entries are read from the local folder and the CDN is never touched. var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); FileSystem.Directory.CreateDirectory(localDir); await FileSystem.File.WriteAllTextAsync( @@ -201,7 +194,7 @@ await FileSystem.File.WriteAllTextAsync( FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); - var handler = RegistryHandler(); + var handler = ProbeHandler(); var service = new ChangelogBundlingService(LoggerFactory, FileSystem, ConfigurationContext, null, Fetcher(Output, handler)); var output = OutputPath(); @@ -255,15 +248,13 @@ public async Task CdnInputProducts_ReturnsError() } [Fact] - public async Task RegistryFailure_FailsBundle() + public async Task CdnIssues_ReturnsError() { - var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), - new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)), sleep: (_, _) => Task.CompletedTask); - var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, fetcher); + var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, Fetcher()); var input = new BundleChangelogsArguments { - Prs = ["https://github.com/elastic/elasticsearch/pull/100"], + Issues = ["https://github.com/elastic/elasticsearch/issues/42"], Output = OutputPath(), Repo = "elasticsearch" }; @@ -271,37 +262,189 @@ public async Task RegistryFailure_FailsBundle() var result = await service.BundleChangelogs(Collector, input, TestContext.Current.CancellationToken); result.Should().BeFalse(); - Collector.Diagnostics.Should().Contain(d => d.Severity == Severity.Error && d.Message.Contains("registry")); + Collector.Diagnostics.Should().Contain(d => + d.Severity == Severity.Error && d.Message.Contains("--issues") && d.Message.Contains("--force-local")); } [Fact] - public async Task EntryMissingAfterRetries_FailsBundle() + public async Task ProbeMiss_WarnsAndSkips() { - // The registry lists two entries but the CDN never serves one of them. After the retry budget is - // spent the bundle must fail rather than silently omit the missing release entry. + // A 404 probe for a PR with no changelog entry warns and skips; the other entry is still included. + // One request per PR (no retries on 404). var handler = new StubHandler(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/registry.json", StringComparison.Ordinal)) - return Json(RegistryJson); - if (path.EndsWith("1-alpha.yaml", StringComparison.Ordinal)) + if (path.EndsWith("/100.yaml", StringComparison.Ordinal)) return Yaml(EntryAlpha); - return new HttpResponseMessage(HttpStatusCode.NotFound); // 2-bravo.yaml never propagates + return new HttpResponseMessage(HttpStatusCode.NotFound); // 999 has no entry }); - var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, maxAttempts: 2, sleep: (_, _) => Task.CompletedTask); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, sleep: (_, _) => Task.CompletedTask); var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, fetcher); + var output = OutputPath(); - var input = new BundleChangelogsArguments + var result = await service.BundleChangelogs(Collector, new BundleChangelogsArguments { Prs = ["https://github.com/elastic/elasticsearch/pull/100", "https://github.com/elastic/elasticsearch/pull/999"], + Output = output, + Repo = "elasticsearch" + }, TestContext.Current.CancellationToken); + + result.Should().BeTrue("probe miss is a warn, not a fatal error"); + Collector.Errors.Should().Be(0); + // 404 is authoritative on the probe path — not retried. + handler.RequestedPaths.Count(p => p.EndsWith("/999.yaml", StringComparison.Ordinal)) + .Should().Be(1, "404 must not be retried on the probe path"); + var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); + bundle.Should().Contain("Alpha", "the entry found by probe must appear in the bundle"); + } + + [Fact] + public async Task ProbedEntry_5xxTransient_Retried() + { + // A transient 5xx on the probe path uses the retry budget; the final content is returned. + var callCount = 0; + var handler = new StubHandler(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.Contains("/100.yaml", StringComparison.Ordinal)) + { + callCount++; + if (callCount < 3) + return new HttpResponseMessage(HttpStatusCode.ServiceUnavailable); + return Yaml(EntryAlpha); + } + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, maxAttempts: 4, sleep: (_, _) => Task.CompletedTask); + var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, fetcher); + + var result = await service.BundleChangelogs(Collector, new BundleChangelogsArguments + { + Prs = ["https://github.com/elastic/elasticsearch/pull/100"], Output = OutputPath(), Repo = "elasticsearch" - }; + }, TestContext.Current.CancellationToken); - var result = await service.BundleChangelogs(Collector, input, TestContext.Current.CancellationToken); + result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); + callCount.Should().BeGreaterThan(1, "5xx must be retried"); + } + + [Fact] + public async Task MarkerEntry_ResolvesDepthOne() + { + // PR 200 is a marker pointing to PR 100 (the primary). Bundling PR 200 should resolve to the Alpha entry. + var handler = new StubHandler(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/100.yaml", StringComparison.Ordinal)) + return Yaml(EntryAlpha); + if (path.EndsWith("/200.yaml", StringComparison.Ordinal)) + return Yaml(MarkerFor(100)); + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, sleep: (_, _) => Task.CompletedTask); + var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, fetcher); + + var output = OutputPath(); + var result = await service.BundleChangelogs(Collector, new BundleChangelogsArguments + { + Prs = ["https://github.com/elastic/elasticsearch/pull/200"], + Output = output, + Repo = "elasticsearch" + }, TestContext.Current.CancellationToken); + + result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); + Collector.Errors.Should().Be(0); + var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); + bundle.Should().Contain("Alpha", "the marker must resolve to its parent entry"); + bundle.Should().NotContain("link:", "the marker itself must not appear in the output"); + } + + [Fact] + public async Task MarkerEntry_DuplicateMarkersToSameParent_OneEntryInBundle() + { + // PRs 200 and 201 are both markers for PR 100. Bundling both should yield one Alpha entry. + var handler = new StubHandler(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/100.yaml", StringComparison.Ordinal)) + return Yaml(EntryAlpha); + if (path.EndsWith("/200.yaml", StringComparison.Ordinal) || path.EndsWith("/201.yaml", StringComparison.Ordinal)) + return Yaml(MarkerFor(100)); + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, sleep: (_, _) => Task.CompletedTask); + var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, fetcher); + + var output = OutputPath(); + var result = await service.BundleChangelogs(Collector, new BundleChangelogsArguments + { + Prs = [ + "https://github.com/elastic/elasticsearch/pull/200", + "https://github.com/elastic/elasticsearch/pull/201" + ], + Output = output, + Repo = "elasticsearch" + }, TestContext.Current.CancellationToken); + + result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); + Collector.Errors.Should().Be(0); + var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); + bundle.Split("title: Alpha").Length.Should().Be(2, "exactly one Alpha entry must appear"); + } + + [Fact] + public async Task MarkerEntry_Chain_FailsBundle() + { + // Marker chains (marker → marker) are hard errors. + var handler = new StubHandler(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/200.yaml", StringComparison.Ordinal)) + return Yaml(MarkerFor(100)); + if (path.EndsWith("/100.yaml", StringComparison.Ordinal)) + return Yaml(MarkerFor(99)); // 100 is also a marker + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, sleep: (_, _) => Task.CompletedTask); + var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, fetcher); + + var result = await service.BundleChangelogs(Collector, new BundleChangelogsArguments + { + Prs = ["https://github.com/elastic/elasticsearch/pull/200"], + Output = OutputPath(), + Repo = "elasticsearch" + }, TestContext.Current.CancellationToken); result.Should().BeFalse(); - Collector.Diagnostics.Should().Contain(d => d.Severity == Severity.Error && d.Message.Contains("2-bravo.yaml")); + Collector.Diagnostics.Should().Contain(d => + d.Severity == Severity.Error && d.Message.Contains("Marker chain")); + } + + [Fact] + public async Task MarkerEntry_ParentMissing_FailsBundle() + { + // A marker whose parent doesn't exist is a hard error (the pipeline promises the parent exists). + var handler = new StubHandler(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/200.yaml", StringComparison.Ordinal)) + return Yaml(MarkerFor(100)); + return new HttpResponseMessage(HttpStatusCode.NotFound); // 100.yaml missing + }); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, sleep: (_, _) => Task.CompletedTask); + var service = new ChangelogBundlingService(LoggerFactory, FileSystem, null, null, fetcher); + + var result = await service.BundleChangelogs(Collector, new BundleChangelogsArguments + { + Prs = ["https://github.com/elastic/elasticsearch/pull/200"], + Output = OutputPath(), + Repo = "elasticsearch" + }, TestContext.Current.CancellationToken); + + result.Should().BeFalse(); + Collector.Diagnostics.Should().Contain(d => + d.Severity == Severity.Error && d.Message.Contains("marker") && d.Message.Contains("100")); } [Fact] From f5dc4c3c8fc7a00058ff0a51edc6ca6f9b4c3f35 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Tue, 25 Aug 2026 18:52:39 +0200 Subject: [PATCH 4/4] style: apply dotnet format to changelog probe sourcing Co-Authored-By: Claude Sonnet 4.6 --- .../Elastic.Changelog/Bundling/ChangelogBundlingService.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index b9f0aecbce..48503454cb 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -293,9 +293,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // On the CDN path entries are probed by key (one per PR), so there is nothing to enumerate without a PR list. if (useCdn && (input.All || input.InputProducts is { Count: > 0 } || input.Issues is { Length: > 0 })) { - var flag = input.All ? "--all" - : input.Issues is { Length: > 0 } ? "--issues" - : "--input-products"; + var flag = input.All ? "--all" : input.Issues is { Length: > 0 } ? "--issues" : "--input-products"; collector.EmitError(string.Empty, $"{flag} is not supported when sourcing changelog entries from the CDN, because entries are fetched by key (one per PR) and there is no pool enumeration. " + "Pass --force-local or --directory to bundle from a local checkout instead.");