From 70457494090a5b99e16c71f35a87940f2f630374 Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Thu, 4 Jun 2026 21:29:10 -0300 Subject: [PATCH 1/2] changelog: source bundle entries from the CDN with a per-product registry Source the individual changelog entries that make up a bundle from the public CDN by default, scoped to the bundle's product(s), instead of the local folder. Because the bundle filter is content-based (an entry's products/prs/issues live inside the YAML, not its name) and CloudFront has no ListObjects, a per-product entry index ({product}/changelog/registry.json) is published on upload so the client can enumerate then fetch+filter. - Add bundle.use_local_changelogs opt-out, plus automatic local fallback when no concrete product can scope the per-product CDN fetch. - Extend RegistryBuilder/RegistryKey to write and pass-through the entry index (scrubber recognizes {product}/changelog/registry.json). - Add CdnChangelogEntryFetcher and centralize CDN base resolution in ChangelogCdn (shared with the changelog directive's cdn: mode). - Emit cdn_url from `changelog bundle --plan` so CI can poll for the scrubbed bundle ({base}/{product}/bundle/{file}). - Harden entry sourcing: a registry-listed entry that has not yet propagated to the CDN is retried with short backoff and cache-busting; a persistent miss fails the bundle instead of silently shipping an incomplete release. dotnet format, AOT publish (0 trim/AOT warnings), and the affected unit tests all pass; cli-schema.json regenerated for the new --plan output. Co-authored-by: Cursor --- docs/cli-schema.json | 2 +- .../Changelog/BundleConfiguration.cs | 8 + .../Changelog/ChangelogConfigurationLoader.cs | 1 + .../Changelog/ChangelogConfigurationYaml.cs | 6 + .../ReleaseNotes/CdnChangelogEntryFetcher.cs | 218 +++++++++++++++ .../Bundling/ChangelogBundlingService.cs | 217 +++++++++++++-- .../Bundling/ChangelogEntryMatcher.cs | 64 ++++- .../Uploading/ChangelogUploadService.cs | 24 +- .../Uploading/RegistryBuilder.cs | 66 +++-- .../Uploading/RegistryKey.cs | 27 +- .../docs-builder/Commands/ChangelogCommand.cs | 4 +- .../Changelogs/BundleCdnSourcingTests.cs | 250 ++++++++++++++++++ .../Changelogs/BundleChangelogsTests.cs | 5 + .../Changelogs/BundlePlanTests.cs | 69 ++++- .../BundleProfileGitHubReleaseTests.cs | 7 + .../Changelogs/ChangelogConfigurationTests.cs | 45 ++++ .../Uploading/RegistryBuilderTests.cs | 31 +++ .../Uploading/RegistryKeyTests.cs | 10 +- .../CdnChangelogEntryFetcherTests.cs | 163 ++++++++++++ 19 files changed, 1157 insertions(+), 60 deletions(-) create mode 100644 src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs create mode 100644 tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs create mode 100644 tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs diff --git a/docs/cli-schema.json b/docs/cli-schema.json index 44e750515d..655d3bb341 100644 --- a/docs/cli-schema.json +++ b/docs/cli-schema.json @@ -2776,7 +2776,7 @@ "name": "plan", "type": "boolean", "required": false, - "summary": "Emit GitHub Actions step outputs (needs_network, needs_github_token, output_path) describing network requirements and the resolved output path, then exit without generating the bundle. Intended for CI actions.", + "summary": "Emit GitHub Actions step outputs (needs_network, needs_github_token, output_path, and cdn_url when a product is resolvable) describing network requirements, the resolved output path, and the public CDN URL of the scrubbed bundle, then exit without generating the bundle. Intended for CI actions.", "defaultValue": "false" }, { diff --git a/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs b/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs index 3191914099..f838ffb8cb 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs @@ -21,6 +21,14 @@ public record BundleConfiguration /// public string? OutputDirectory { get; init; } + /// + /// When true, the individual changelog entries that make up a bundle are sourced from the local + /// . When false (the default), they are fetched from the public changelog + /// CDN, scoped to the bundle's products. An explicit --directory on the CLI always forces + /// local sourcing regardless of this setting. + /// + public bool UseLocalChangelogs { get; init; } + /// /// Whether to resolve (copy contents of each changelog file into the entries array). /// Defaults to true diff --git a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs index 38a73ee728..2106bf52d8 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs @@ -529,6 +529,7 @@ private static PivotConfiguration ConvertPivot(PivotConfigurationYaml yamlPivot) { Directory = yaml.Directory, OutputDirectory = yaml.OutputDirectory, + UseLocalChangelogs = yaml.UseLocalChangelogs ?? false, Resolve = yaml.Resolve ?? true, Description = yaml.Description, Repo = yaml.Repo, diff --git a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs index 80521f6742..41f628acce 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs @@ -275,6 +275,12 @@ internal sealed record BundleConfigurationYaml /// public string? OutputDirectory { get; set; } + /// + /// When true, source the individual changelog entries that make up a bundle from the local + /// instead of the public CDN. Defaults to false (CDN sourcing). + /// + public bool? UseLocalChangelogs { get; set; } + /// /// Whether to resolve (copy contents) by default. /// diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs new file mode 100644 index 0000000000..f51833945a --- /dev/null +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs @@ -0,0 +1,218 @@ +// 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 System.Text.Json; +using Microsoft.Extensions.Logging; + +namespace Elastic.Documentation.Configuration.ReleaseNotes; + +/// +/// One downloaded changelog entry: the registry file name and its raw YAML content. +/// +public readonly record struct CdnChangelogEntry(string FileName, string Content); + +/// +/// Fetches the individual (scrubbed) changelog entries for a single product from the public CDN, for +/// the changelog bundle command when sourcing entries from S3 rather than a local folder. It +/// reads {base}/{product}/changelog/registry.json to enumerate entries and downloads each +/// {base}/{product}/changelog/{file} as raw YAML; the bundle command then applies its usual +/// filter (products / prs / issues) to the downloaded set. +/// +/// +/// +/// A registry that cannot be fetched or parsed is a hard error (the caller gets an empty list and an +/// emitted error). An individual entry that the registry lists but the CDN does not yet serve is +/// retried a few times with short backoff (and cache-busting, to defeat any CloudFront negative-cache) +/// to ride out the brief upload→scrub→propagate window. If it still cannot be fetched after the retry +/// budget it is escalated to an error, not skipped: the registry asserts the entry exists (uploads +/// never prune) and scrubbing is sub-second, so a persistent miss is a real pipeline problem and +/// silently shipping an incomplete release bundle is worse than failing the run. +/// +/// +public sealed class CdnChangelogEntryFetcher( + ILoggerFactory logFactory, + HttpMessageHandler? handler = null, + int maxAttempts = CdnChangelogEntryFetcher.DefaultMaxAttempts, + Action? sleep = null) +{ + private const int SupportedSchemaVersion = 1; + + /// Total GET attempts per entry (1 initial + retries). ~3.5s budget at the default backoff. + private const int DefaultMaxAttempts = 4; + private const int BaseRetryDelayMs = 500; + private const int MaxRetryDelayMs = 2000; + + private readonly ILogger _logger = logFactory.CreateLogger(); + private readonly HttpClient _httpClient = handler is null ? new HttpClient() : new HttpClient(handler, disposeHandler: false); + private readonly int _maxAttempts = maxAttempts < 1 ? DefaultMaxAttempts : maxAttempts; + private readonly Action _sleep = sleep ?? DefaultSleep; + + /// + /// Downloads the changelog entries for from the CDN at + /// . Returns an empty list after emitting an error when the registry cannot + /// be read or when a registry-listed entry cannot be fetched within the retry budget. Entries are + /// returned in registry order; the caller owns filtering and de-duplication. + /// + public IReadOnlyList Fetch( + Uri baseUri, + string product, + Action emitError, + Action emitWarning, + Cancel ctx) + { + var registryUri = Combine(baseUri, product, "changelog", "registry.json"); + + ChangelogRegistry? registry; + try + { + registry = FetchRegistry(registryUri, ctx); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + emitError($"Could not fetch changelog entry registry for product '{product}' from {registryUri}: {ex.Message}"); + return []; + } + + if (registry is null) + { + emitError($"Changelog entry registry for product '{product}' at {registryUri} was empty or unparseable."); + return []; + } + + if (registry.SchemaVersion > SupportedSchemaVersion) + { + emitError( + $"Changelog entry registry for product '{product}' uses schema version {registry.SchemaVersion}, but this build only understands version {SupportedSchemaVersion}. Update docs-builder."); + return []; + } + + var entries = new List(registry.Bundles.Count); + foreach (var entry in registry.Bundles) + { + ctx.ThrowIfCancellationRequested(); + + var fileName = entry.File; + if (string.IsNullOrWhiteSpace(fileName) || !IsSafeFileName(fileName)) + { + emitWarning($"Changelog entry registry for '{product}' lists an invalid file name '{fileName}'; skipping."); + continue; + } + + var entryUri = Combine(baseUri, product, "changelog", fileName); + if (TryFetchEntry(entryUri, fileName, product, ctx, out var content, out var lastError)) + { + entries.Add(new CdnChangelogEntry(fileName, content)); + continue; + } + + // The registry lists this entry, so it exists in the private bucket and should have been + // scrubbed to the public one within milliseconds. Still missing after the retry budget means + // a genuine propagation/scrub failure — fail rather than ship a bundle missing this entry. + emitError( + $"Changelog entry '{fileName}' for product '{product}' is listed in the registry but could not be fetched from {entryUri} after {_maxAttempts} attempt(s): {lastError}. " + + "The scrubbed copy may not have propagated to the CDN yet; retry shortly, and if it persists check the changelog scrubber pipeline."); + return []; + } + + _logger.LogInformation("Fetched {Count} changelog entry(ies) for {Product} from {BaseUri}", entries.Count, product, baseUri); + return entries; + } + + /// + /// 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 + /// so a CloudFront-cached 404 cannot pin the result for the whole window. + /// + private bool TryFetchEntry(Uri uri, string fileName, string product, Cancel ctx, out string content, out string? lastError) + { + content = string.Empty; + lastError = null; + + for (var attempt = 1; attempt <= _maxAttempts; attempt++) + { + ctx.ThrowIfCancellationRequested(); + try + { + content = FetchText(uri, attempt, ctx); + if (attempt > 1) + _logger.LogInformation("Fetched changelog entry '{File}' for {Product} on attempt {Attempt}/{Max}", fileName, product, attempt, _maxAttempts); + return true; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + lastError = ex.Message; + if (attempt >= _maxAttempts) + break; + + var delay = RetryDelay(attempt); + _logger.LogDebug( + "Changelog entry '{File}' for {Product} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", + fileName, product, attempt, _maxAttempts, ex.Message, delay); + _sleep(delay, ctx); + } + } + + return false; + } + + private ChangelogRegistry? FetchRegistry(Uri registryUri, Cancel ctx) + { + _logger.LogInformation("Fetching changelog entry registry {RegistryUri}", registryUri); + using var request = new HttpRequestMessage(HttpMethod.Get, registryUri); + using var response = _httpClient.Send(request, ctx); + _ = response.EnsureSuccessStatusCode(); + using var stream = response.Content.ReadAsStream(ctx); + return JsonSerializer.Deserialize(stream, ChangelogRegistryJsonContext.Default.ChangelogRegistry); + } + + private string FetchText(Uri uri, int attempt, Cancel ctx) + { + // Only bust the cache on retries: the first hit should use the CDN cache normally (the common, + // already-propagated case); retries explicitly want to bypass any cached 404. + var requestUri = attempt > 1 ? WithCacheBuster(uri) : uri; + using var request = new HttpRequestMessage(HttpMethod.Get, requestUri); + if (attempt > 1) + _ = request.Headers.TryAddWithoutValidation("Cache-Control", "no-cache"); + using var response = _httpClient.Send(request, ctx); + _ = response.EnsureSuccessStatusCode(); + using var stream = response.Content.ReadAsStream(ctx); + using var reader = new StreamReader(stream); + return reader.ReadToEnd(); + } + + private static TimeSpan RetryDelay(int attempt) + { + // attempt is 1-based; first retry waits BaseRetryDelayMs, doubling up to the cap. + var ms = Math.Min(BaseRetryDelayMs * (1L << (attempt - 1)), MaxRetryDelayMs); + return TimeSpan.FromMilliseconds(ms); + } + + private static void DefaultSleep(TimeSpan delay, Cancel ctx) + { + if (delay > TimeSpan.Zero) + _ = ctx.WaitHandle.WaitOne(delay); + } + + private static Uri WithCacheBuster(Uri uri) + { + var separator = string.IsNullOrEmpty(uri.Query) ? "?" : "&"; + return new Uri($"{uri.AbsoluteUri}{separator}_={DateTimeOffset.UtcNow.Ticks:x}"); + } + + private static Uri Combine(Uri baseUri, params string[] segments) + { + var basePath = baseUri.AbsoluteUri.TrimEnd('/'); + var suffix = string.Join('/', segments.Select(Uri.EscapeDataString)); + return new Uri($"{basePath}/{suffix}"); + } + + /// + /// Guards against path traversal or nested keys sneaking in via the registry: an entry file name + /// must be a single path segment (the producer always writes {product}/changelog/{file}). + /// + private static bool IsSafeFileName(string fileName) => + !fileName.Contains('/', StringComparison.Ordinal) + && !fileName.Contains('\\', StringComparison.Ordinal) + && fileName is not ("." or ".."); +} diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index cf0b061e21..ed7c8eed88 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -118,6 +118,13 @@ public record BundlePlanResult public bool NeedsNetwork { get; init; } public bool NeedsGithubToken { get; init; } public string? OutputPath { get; init; } + + /// + /// Public CDN URL of the (scrubbed) bundle once uploaded: {base}/{product}/bundle/{file}. + /// Null when no concrete product can be resolved to scope the URL (e.g. option-mode PR/issue-only + /// filters). Consumed by the bundle-PR action to poll for and download the scrubbed copy. + /// + public string? CdnUrl { get; init; } } /// @@ -127,12 +134,14 @@ public partial class ChangelogBundlingService( ILoggerFactory logFactory, IConfigurationContext? configurationContext = null, ScopedFileSystem? fileSystem = null, - IGitHubReleaseService? releaseService = null) + IGitHubReleaseService? releaseService = null, + CdnChangelogEntryFetcher? entryFetcher = null) : IService { private readonly ILogger _logger = logFactory.CreateLogger(); private readonly ScopedFileSystem _fileSystem = fileSystem ?? FileSystemFactory.RealRead; private readonly IGitHubReleaseService _releaseService = releaseService ?? new GitHubReleaseService(logFactory); + private readonly CdnChangelogEntryFetcher _entryFetcher = entryFetcher ?? new CdnChangelogEntryFetcher(logFactory); private readonly ChangelogConfigurationLoader? _configLoader = configurationContext != null ? new ChangelogConfigurationLoader(logFactory, configurationContext, fileSystem ?? FileSystemFactory.RealRead) : null; @@ -155,6 +164,10 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle { try { + // Capture whether the caller explicitly pointed at a local folder before config defaults + // fill it in. An explicit --directory always forces local sourcing. + var explicitDirectory = !string.IsNullOrWhiteSpace(input.Directory); + // Load changelog configuration ChangelogConfiguration? config = null; if (!string.IsNullOrWhiteSpace(input.Profile)) @@ -197,8 +210,18 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // Apply config defaults if available input = ApplyConfigDefaults(input, config); - // Validate input - if (!ValidateInput(collector, input)) + // Decide where the individual changelog entries come from. Default is the public CDN, scoped + // to the bundle's products. Fall back to local folder sourcing when the user opted in + // (bundle.use_local_changelogs), passed an explicit --directory, or when no concrete product + // can be resolved to scope the per-product CDN fetch (e.g. an option-mode PR/issue-only + // filter). This keeps the run in lockstep with PlanBundleAsync's needs_network decision. + var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; + var cdnProducts = ResolveCdnProducts(input); + var useCdn = !useLocalChangelogs && !explicitDirectory && cdnProducts.Count > 0; + + // Validate input. In CDN mode the local input directory is not read, so its existence + // is not required. + if (!ValidateInput(collector, input, requireDirectoryExists: !useCdn)) return false; if (!ValidatePlaceholderUsage(collector, input)) @@ -234,24 +257,34 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // Determine output path var outputPath = input.Output ?? _fileSystem.Path.Join(directory, "changelog-bundle.yaml"); - // Discover changelog files - var fileDiscovery = new ChangelogFileDiscovery(_fileSystem, _logger); - var yamlFiles = await fileDiscovery.DiscoverChangelogFilesAsync(directory, outputPath, ctx); + // Build filter criteria + var filterCriteria = BuildFilterCriteria(input, prsToMatch, issuesToMatch); - if (yamlFiles.Count == 0) + // Source and match changelog entries — from the CDN (default) or the local folder. + var entryMatcher = new ChangelogEntryMatcher(_fileSystem, ReleaseNotesSerialization.GetEntryDeserializer(), _logger); + ChangelogMatchResult matchResult; + if (useCdn) { - collector.EmitError(directory, "No YAML files found in directory"); - return false; + var contents = FetchCdnEntries(collector, cdnProducts, ctx); + if (contents == null) + return false; + matchResult = entryMatcher.MatchChangelogContents(collector, contents, filterCriteria, ctx); } + else + { + // Discover changelog files + var fileDiscovery = new ChangelogFileDiscovery(_fileSystem, _logger); + var yamlFiles = await fileDiscovery.DiscoverChangelogFilesAsync(directory, outputPath, ctx); - _logger.LogInformation("Found {Count} YAML files in directory", yamlFiles.Count); - - // Build filter criteria - var filterCriteria = BuildFilterCriteria(input, prsToMatch, issuesToMatch); + if (yamlFiles.Count == 0) + { + collector.EmitError(directory, "No YAML files found in directory"); + return false; + } - // Match changelog entries - var entryMatcher = new ChangelogEntryMatcher(_fileSystem, ReleaseNotesSerialization.GetEntryDeserializer(), _logger); - var matchResult = await entryMatcher.MatchChangelogsAsync(collector, yamlFiles, filterCriteria, ctx); + _logger.LogInformation("Found {Count} YAML files in directory", yamlFiles.Count); + matchResult = await entryMatcher.MatchChangelogsAsync(collector, yamlFiles, filterCriteria, ctx); + } _logger.LogInformation("Found {Count} matching changelog entries", matchResult.Entries.Count); @@ -611,6 +644,18 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } } + // CDN entry sourcing (the default) needs network access for the Docker bundle run. It is active + // unless the user opted into local sourcing (bundle.use_local_changelogs) or passed --directory, + // and only when a product can be resolved to scope the fetch. + var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; + var explicitDirectory = !string.IsNullOrWhiteSpace(input.Directory); + var cdnProductsResolvable = !string.IsNullOrWhiteSpace(input.Profile) + ? profileDef is not null && + (!string.IsNullOrWhiteSpace(profileDef.Products) || !string.IsNullOrWhiteSpace(profileDef.OutputProducts)) + : input.InputProducts is { Count: > 0 } || input.OutputProducts is { Count: > 0 }; + if (!useLocalChangelogs && !explicitDirectory && cdnProductsResolvable) + needsNetwork = true; + // Resolve output path — mirrors the logic in ProcessProfile + ApplyConfigDefaults. var outputPath = input.Output; if (string.IsNullOrWhiteSpace(outputPath) && profileDef?.Output != null) @@ -632,11 +677,145 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments { NeedsNetwork = needsNetwork, NeedsGithubToken = needsGithubToken, - OutputPath = outputPath + OutputPath = outputPath, + CdnUrl = ResolveCdnBundleUrl(profileDef, input, outputPath) }; } - private bool ValidateInput(IDiagnosticsCollector collector, BundleChangelogsArguments input) + /// + /// Builds the public CDN URL where the scrubbed bundle will be served once uploaded: + /// {base}/{product}/bundle/{file}. The bundle-PR action polls this to fetch the scrubbed copy + /// rather than committing the raw, locally-generated file. Returns null when the product, output file + /// name, or CDN base cannot be resolved. + /// + private string? ResolveCdnBundleUrl(BundleProfile? profileDef, BundleChangelogsArguments input, string? outputPath) + { + if (string.IsNullOrWhiteSpace(outputPath)) + return null; + + var product = ResolvePrimaryProduct(profileDef, input); + if (string.IsNullOrWhiteSpace(product)) + return null; + + if (ChangelogCdn.ResolveBaseUri() is not { } baseUri) + return null; + + var fileName = _fileSystem.Path.GetFileName(outputPath); + if (string.IsNullOrWhiteSpace(fileName)) + return null; + + var basePath = baseUri.AbsoluteUri.TrimEnd('/'); + return $"{basePath}/{Uri.EscapeDataString(product)}/bundle/{Uri.EscapeDataString(fileName)}"; + } + + /// + /// The first concrete (non-wildcard) product that scopes the bundle, used to build its CDN URL. Taken + /// from the profile's output_products/products pattern (first whitespace-delimited token + /// of the first comma-group) or, in option mode, the first explicit output/input product argument. + /// + private static string? ResolvePrimaryProduct(BundleProfile? profileDef, BundleChangelogsArguments input) + { + var pattern = profileDef?.OutputProducts ?? profileDef?.Products; + if (!string.IsNullOrWhiteSpace(pattern)) + { + var firstGroup = pattern.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries).FirstOrDefault(); + var id = firstGroup?.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault(); + if (!string.IsNullOrWhiteSpace(id) && id != "*") + return id; + } + + foreach (var list in new[] { input.OutputProducts, input.InputProducts }) + { + if (list is null) + continue; + foreach (var p in list) + { + if (!string.IsNullOrWhiteSpace(p.Product) && p.Product != "*") + return p.Product; + } + } + + return null; + } + + /// + /// Downloads the in-scope changelog entries from the public CDN, scoped to the bundle's products. + /// Returns null (after emitting an error) when no product can be resolved or a per-product + /// registry cannot be fetched; an individual entry that is not yet public is skipped with a warning. + /// + private IReadOnlyList<(string FileName, string Content)>? FetchCdnEntries( + IDiagnosticsCollector collector, + IReadOnlyList products, + Cancel ctx) + { + if (products.Count == 0) + { + collector.EmitError(string.Empty, + "Sourcing changelog entries from the CDN requires a resolvable product. Use a profile or " + + "--input-products with a concrete product, set bundle.use_local_changelogs: true in " + + "changelog.yml, or pass --directory to bundle local changelog files."); + return null; + } + + 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 byName = new Dictionary(StringComparer.Ordinal); + var fatalFailure = false; + foreach (var product in products) + { + var entries = _entryFetcher.Fetch( + baseUri, + product, + msg => { fatalFailure = true; collector.EmitError(string.Empty, msg); }, + msg => collector.EmitWarning(string.Empty, msg), + ctx); + + foreach (var entry in entries) + byName[entry.FileName] = entry.Content; + } + + // The fetcher emits an error (via the callback above) for any fatal condition — a registry that + // cannot be read, or a registry-listed entry still missing after its retry budget. Either would + // silently drop entries and ship an incomplete bundle, so treat it as fatal. + if (fatalFailure) + return null; + + _logger.LogInformation("Sourced {Count} changelog entr(ies) from the CDN for product(s) {Products}", + byName.Count, string.Join(", ", products)); + + return byName.Select(kv => (kv.Key, kv.Value)).ToList(); + } + + /// + /// The distinct, concrete product IDs that scope a CDN-sourced bundle, taken from the resolved + /// input products (profile / --input-products) and any explicit output products. Wildcards are excluded. + /// + private static IReadOnlyList ResolveCdnProducts(BundleChangelogsArguments input) + { + var ids = new List(); + AppendConcreteProducts(ids, input.InputProducts); + AppendConcreteProducts(ids, input.OutputProducts); + return ids.Distinct(StringComparer.Ordinal).ToList(); + } + + private static void AppendConcreteProducts(List ids, IReadOnlyList? products) + { + if (products == null) + return; + foreach (var product in products) + { + if (!string.IsNullOrWhiteSpace(product.Product) && product.Product != "*") + ids.Add(product.Product); + } + } + + private bool ValidateInput(IDiagnosticsCollector collector, BundleChangelogsArguments input, bool requireDirectoryExists) { if (string.IsNullOrWhiteSpace(input.Directory)) { @@ -644,7 +823,7 @@ private bool ValidateInput(IDiagnosticsCollector collector, BundleChangelogsArgu return false; } - if (!_fileSystem.Directory.Exists(input.Directory)) + if (requireDirectoryExists && !_fileSystem.Directory.Exists(input.Directory)) { collector.EmitError(input.Directory, "Directory does not exist"); return false; diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogEntryMatcher.cs b/src/services/Elastic.Changelog/Bundling/ChangelogEntryMatcher.cs index 52eb92c62c..56f9f259ab 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogEntryMatcher.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogEntryMatcher.cs @@ -39,6 +39,43 @@ public async Task MatchChangelogsAsync( changelogEntries.Add(entry); } + return BuildResult(collector, changelogEntries, criteria, matchedPrs, matchedIssues); + } + + /// + /// Parses and filters changelog entries supplied as in-memory (file name, content) pairs, e.g. + /// when sourcing entries from the CDN. Behaves identically to + /// but reads no files; the file name is used as the entry's identity (path, name, and references). + /// + public ChangelogMatchResult MatchChangelogContents( + IDiagnosticsCollector collector, + IReadOnlyList<(string FileName, string Content)> contents, + ChangelogFilterCriteria criteria, + Cancel ctx) + { + var changelogEntries = new List(); + var matchedPrs = new HashSet(StringComparer.OrdinalIgnoreCase); + var matchedIssues = new HashSet(StringComparer.OrdinalIgnoreCase); + var seenChangelogs = new HashSet(); + + foreach (var (fileName, content) in contents) + { + ctx.ThrowIfCancellationRequested(); + var entry = ProcessContent(collector, fileName, fileName, content, criteria, seenChangelogs, matchedPrs, matchedIssues); + if (entry != null) + changelogEntries.Add(entry); + } + + return BuildResult(collector, changelogEntries, criteria, matchedPrs, matchedIssues); + } + + private static ChangelogMatchResult BuildResult( + IDiagnosticsCollector collector, + List changelogEntries, + ChangelogFilterCriteria criteria, + HashSet matchedPrs, + HashSet matchedIssues) + { if (criteria.PrsToMatch.Count > 0) { foreach (var pr in criteria.PrsToMatch.Where(pr => !matchedPrs.Contains(pr))) @@ -68,11 +105,34 @@ public async Task MatchChangelogsAsync( HashSet matchedIssues, Cancel ctx) { + string fileContent; try { - var fileName = fileSystem.Path.GetFileName(filePath); - var fileContent = await fileSystem.File.ReadAllTextAsync(filePath, ctx); + fileContent = await fileSystem.File.ReadAllTextAsync(filePath, ctx); + } + catch (Exception ex) when (ex is not (OperationCanceledException or OutOfMemoryException or StackOverflowException or ThreadAbortException)) + { + logger.LogWarning(ex, "Error reading file {FilePath}", filePath); + collector.EmitError(filePath, $"Error processing file: {ex.Message}"); + return null; + } + + var fileName = fileSystem.Path.GetFileName(filePath); + return ProcessContent(collector, filePath, fileName, fileContent, criteria, seenChangelogs, matchedPrs, matchedIssues); + } + private MatchedChangelogFile? ProcessContent( + IDiagnosticsCollector collector, + string filePath, + string fileName, + string fileContent, + ChangelogFilterCriteria criteria, + HashSet seenChangelogs, + HashSet matchedPrs, + HashSet matchedIssues) + { + try + { // Compute checksum (SHA1) var checksum = ChangelogBundlingService.ComputeSha1(fileContent); diff --git a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs index 69761a1560..f3f2f70af2 100644 --- a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs +++ b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs @@ -94,11 +94,16 @@ public async Task Upload(IDiagnosticsCollector collector, ChangelogUploadA if (result.Failed > 0) collector.EmitError(string.Empty, $"{result.Failed} file(s) failed to upload"); - // On a successful bundle upload, refresh the per-product registry.json so consumers - // (e.g. the changelog directive in cdn: mode) can enumerate bundles without an S3 listing. - // Failures here are logged but don't fail the upload — the bundles themselves are already in S3. - if (result.Failed == 0 && args.ArtifactType == ArtifactType.Bundle && targets.Count > 0) - await RefreshRegistries(collector, client, etagCalculator, args, targets, ctx); + // On a successful upload, refresh the per-product registry.json so consumers can enumerate + // content without an S3 listing: the bundle index (consumed by the changelog directive in + // cdn: mode) for bundle uploads, and the changelog-entry index (consumed by `changelog + // bundle` when sourcing entries from the CDN) for changelog uploads. + // Failures here are logged but don't fail the upload — the objects themselves are already in S3. + if (result.Failed == 0 && targets.Count > 0) + { + var scope = args.ArtifactType == ArtifactType.Bundle ? RegistryScope.Bundle : RegistryScope.Changelog; + await RefreshRegistries(collector, client, etagCalculator, args, targets, scope, ctx); + } return result.Failed == 0; } @@ -108,15 +113,16 @@ private async Task RefreshRegistries( IAmazonS3 client, IS3EtagCalculator etagCalculator, ChangelogUploadArguments args, - IReadOnlyList bundleTargets, + IReadOnlyList uploadTargets, + RegistryScope scope, Cancel ctx) { try { var builder = new RegistryBuilder(logFactory, _fileSystem, client, etagCalculator, args.S3BucketName); - var result = await builder.RefreshAsync(collector, bundleTargets, ctx); - _logger.LogInformation("Registry refresh: {Updated} updated, {Unchanged} unchanged, {Failed} failed", - result.Updated, result.Unchanged, result.Failed); + var result = await builder.RefreshAsync(collector, uploadTargets, ctx, scope); + _logger.LogInformation("Registry refresh ({Scope}): {Updated} updated, {Unchanged} unchanged, {Failed} failed", + scope, result.Updated, result.Unchanged, result.Failed); } catch (Exception ex) when (ex is not OperationCanceledException) { diff --git a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs index aa47d833d7..f857d7ea4a 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs @@ -16,10 +16,24 @@ namespace Elastic.Changelog.Uploading; /// -/// Refreshes the per-product {product}/registry.json manifest in the private bundles -/// bucket after a bundle upload run. Each product touched in the run gets its manifest merged with -/// the bundles already known on S3 (read back, merged by file name, written with an optimistic -/// concurrency guard so parallel uploads for the same product cannot clobber each other). +/// Which per-product manifest a run refreshes. +/// +internal enum RegistryScope +{ + /// The bundle index at {product}/registry.json, listing scrubbed bundle files. + Bundle, + + /// The changelog-entry index at {product}/changelog/registry.json, listing individual entry files. + Changelog +} + +/// +/// Refreshes a per-product registry.json manifest in the private bucket after an upload run. +/// Depending on this is either the bundle index +/// ({product}/registry.json) or the changelog-entry index +/// ({product}/changelog/registry.json). Each product touched in the run gets its manifest +/// merged with what is already known on S3 (read back, merged by file name, written with an +/// optimistic concurrency guard so parallel uploads for the same product cannot clobber each other). /// internal sealed class RegistryBuilder( ILoggerFactory logFactory, @@ -47,16 +61,18 @@ internal sealed record RefreshResult(int Updated, int Unchanged, int Failed); /// concurrent writer won the race, so we re-read, re-merge, and retry. /// /// Diagnostics sink for non-fatal warnings. - /// Upload targets produced by DiscoverBundleUploadTargets. + /// Upload targets produced by DiscoverBundleUploadTargets or DiscoverUploadTargets. /// Cancellation token. + /// Which per-product manifest to refresh (bundle index or changelog-entry index). public async Task RefreshAsync( IDiagnosticsCollector collector, - IReadOnlyList bundleTargets, - Cancel ctx) + IReadOnlyList uploadTargets, + Cancel ctx, + RegistryScope scope = RegistryScope.Bundle) { - // Each upload target carries a "{product}/bundle/{file}" S3 key. Group by product + // Each upload target carries a "{product}/{bundle|changelog}/{file}" S3 key. Group by product // so we can produce one manifest per affected product. - var byProduct = bundleTargets + var byProduct = uploadTargets .Select(t => (Target: t, Product: ExtractProduct(t.S3Key))) .Where(x => x.Product is not null) .GroupBy(x => x.Product!, StringComparer.Ordinal); @@ -70,14 +86,14 @@ public async Task RefreshAsync( ctx.ThrowIfCancellationRequested(); var product = group.Key; - var localEntries = await BuildLocalEntries(collector, product, group.Select(x => x.Target).ToList(), ctx); + var localEntries = await BuildLocalEntries(collector, product, group.Select(x => x.Target).ToList(), scope, ctx); if (localEntries.Count == 0) { _logger.LogDebug("No usable manifest entries derived for product {Product}; skipping", product); continue; } - switch (await WriteManifest(collector, product, localEntries, ctx)) + switch (await WriteManifest(collector, product, localEntries, scope, ctx)) { case WriteOutcome.Updated: updated++; @@ -96,7 +112,8 @@ public async Task RefreshAsync( /// /// Extracts the leading product segment from an S3 key shaped like - /// {product}/bundle/{file}. Returns null on unrecognized shapes. + /// {product}/bundle/{file} or {product}/changelog/{file}. Returns null on + /// unrecognized shapes. /// private static string? ExtractProduct(string s3Key) { @@ -106,6 +123,15 @@ public async Task RefreshAsync( return s3Key.AsSpan(0, firstSlash).ToString(); } + /// + /// The S3 key of the per-product manifest for the given . + /// + private static string RegistryKeyFor(string product, RegistryScope scope) => scope switch + { + RegistryScope.Changelog => $"{product}/changelog/registry.json", + _ => $"{product}/registry.json" + }; + /// /// Computes manifest entries for the bundles uploaded in this run by reading their YAML /// and computing the S3 ETag locally. The target is taken from the bundle's declaration of @@ -116,6 +142,7 @@ private async Task> BuildLocalEntries( IDiagnosticsCollector collector, string product, IReadOnlyList targets, + RegistryScope scope, Cancel ctx) { var entries = new List(targets.Count); @@ -123,7 +150,11 @@ private async Task> BuildLocalEntries( { ctx.ThrowIfCancellationRequested(); - var targetVersion = ReadTargetForProduct(collector, target.LocalPath, product); + // The changelog-entry index only needs to enumerate files (consumers re-read each entry + // to filter), so target is left unset there; the bundle index records the per-product target. + var targetVersion = scope == RegistryScope.Bundle + ? ReadTargetForProduct(collector, target.LocalPath, product) + : null; string etag; try @@ -174,15 +205,16 @@ private async Task WriteManifest( IDiagnosticsCollector collector, string product, IReadOnlyList localEntries, + RegistryScope scope, Cancel ctx) { - var key = $"{product}/registry.json"; + var key = RegistryKeyFor(product, scope); for (var attempt = 1; attempt <= MaxWriteAttempts; attempt++) { ctx.ThrowIfCancellationRequested(); - var (existing, etag) = await TryFetchExistingManifest(product, ctx); + var (existing, etag) = await TryFetchExistingManifest(product, scope, ctx); var merged = Merge(existing, localEntries); // Re-uploading the same bundles must not churn the manifest (keeps reruns idempotent). @@ -224,9 +256,9 @@ private async Task WriteManifest( /// write). Returns an empty list with a null ETag when the object does not exist. A corrupt object /// returns an empty list with the live ETag so the retry loop can conditionally overwrite it. /// - private async Task<(IReadOnlyList Bundles, string? ETag)> TryFetchExistingManifest(string product, Cancel ctx) + private async Task<(IReadOnlyList Bundles, string? ETag)> TryFetchExistingManifest(string product, RegistryScope scope, Cancel ctx) { - var key = $"{product}/registry.json"; + var key = RegistryKeyFor(product, scope); string? etag = null; try { diff --git a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs index aa2d235294..e25519b493 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs @@ -6,17 +6,19 @@ namespace Elastic.Changelog.Uploading; /// /// Helpers for validating S3 object keys related to the per-product -/// manifest. +/// manifests (the bundle index and the changelog-entry index). /// public static class RegistryKey { - private const string Suffix = "/registry.json"; + private const string BundleSuffix = "/registry.json"; + private const string ChangelogSuffix = "/changelog/registry.json"; /// - /// Returns true when is a top-level per-product manifest of the - /// form {product}/registry.json, where {product} matches the same - /// character class enforced by ChangelogUploadService.ProductNameRegex - /// ([a-zA-Z0-9_-]+). + /// Returns true when is a per-product manifest of either form + /// {product}/registry.json (the bundle index) or + /// {product}/changelog/registry.json (the changelog-entry index), where + /// {product} matches the same character class enforced by + /// ChangelogUploadService.ProductNameRegex ([a-zA-Z0-9_-]+). /// /// /// Used by the changelog scrubber Lambda to decide whether to pass an incoming @@ -29,10 +31,17 @@ public static bool IsRegistry(string key) if (string.IsNullOrEmpty(key)) return false; - if (!key.EndsWith(Suffix, StringComparison.Ordinal)) - return false; + if (key.EndsWith(ChangelogSuffix, StringComparison.Ordinal)) + return IsValidProduct(key.AsSpan(0, key.Length - ChangelogSuffix.Length)); + + if (key.EndsWith(BundleSuffix, StringComparison.Ordinal)) + return IsValidProduct(key.AsSpan(0, key.Length - BundleSuffix.Length)); - var product = key.AsSpan(0, key.Length - Suffix.Length); + return false; + } + + private static bool IsValidProduct(ReadOnlySpan product) + { if (product.IsEmpty || product.Contains('/')) return false; diff --git a/src/tooling/docs-builder/Commands/ChangelogCommand.cs b/src/tooling/docs-builder/Commands/ChangelogCommand.cs index 860bd26074..74e6cfd6b9 100644 --- a/src/tooling/docs-builder/Commands/ChangelogCommand.cs +++ b/src/tooling/docs-builder/Commands/ChangelogCommand.cs @@ -563,7 +563,7 @@ async static (s, collector, state, ctx) => await s.CreateChangelog(collector, st /// A URL or file path to a promotion report. Extracts PR URLs and uses them as the filter. /// GitHub release tag to use as a filter source (for example, "v9.2.0" or "latest"). When specified, fetches the release, parses PR references from the release notes, and uses those PRs as the filter — equivalent to passing the PR list via --prs. When --output-products is not specified, it is inferred from the release tag and repository name. /// Optional: Copy the contents of each changelog file into the entries array. Uses config bundle.resolve or defaults to false. - /// Emit GitHub Actions step outputs (needs_network, needs_github_token, output_path) describing network requirements and the resolved output path, then exit without generating the bundle. Intended for CI actions. + /// Emit GitHub Actions step outputs (needs_network, needs_github_token, output_path, and cdn_url when a product is resolvable) describing network requirements, the resolved output path, and the public CDN URL of the scrubbed bundle, then exit without generating the bundle. Intended for CI actions. /// [NoOptionsInjection] public async Task Bundle( @@ -842,6 +842,8 @@ public async Task Bundle( await githubActionsService.SetOutputAsync("needs_github_token", planResult.NeedsGithubToken ? "true" : "false"); if (planResult.OutputPath != null) await githubActionsService.SetOutputAsync("output_path", planResult.OutputPath); + if (planResult.CdnUrl != null) + await githubActionsService.SetOutputAsync("cdn_url", planResult.CdnUrl); return 0; } diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs new file mode 100644 index 0000000000..faea3af32e --- /dev/null +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs @@ -0,0 +1,250 @@ +// 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 System.Net; +using AwesomeAssertions; +using Elastic.Changelog.Bundling; +using Elastic.Changelog.GitHub; +using Elastic.Documentation.Configuration; +using Elastic.Documentation.Configuration.ReleaseNotes; +using Elastic.Documentation.Diagnostics; +using FakeItEasy; + +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). +/// +public class BundleCdnSourcingTests(ITestOutputHelper output) : ChangelogTestBase(output) +{ + // language=yaml + private const string EntryAlpha = """ + title: Alpha + type: feature + products: + - product: elasticsearch + target: 9.3.0 + lifecycle: ga + prs: + - https://github.com/elastic/elasticsearch/pull/100 + """; + + // language=yaml + private const string EntryBravo = """ + title: Bravo + type: feature + products: + - product: elasticsearch + target: 9.3.0 + lifecycle: ga + prs: + - 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" } ] }"""; + + private static StubHandler RegistryHandler() => new(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + return Json(RegistryJson); + if (path.EndsWith("1-alpha.yaml", StringComparison.Ordinal)) + return Yaml(EntryAlpha); + if (path.EndsWith("2-bravo.yaml", StringComparison.Ordinal)) + return Yaml(EntryBravo); + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + + // No-op sleeper so any entry retry stays instant in tests. + private static CdnChangelogEntryFetcher Fetcher(ITestOutputHelper output, StubHandler handler) => + new(new TestLoggerFactory(output), handler, sleep: (_, _) => { }); + + private CdnChangelogEntryFetcher Fetcher() => Fetcher(Output, RegistryHandler()); + + private string OutputPath() => + FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "bundle.yaml"); + + [Fact] + public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() + { + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher()); + var output = OutputPath(); + + var input = new BundleChangelogsArguments + { + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = output, + Resolve = true + }; + + 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))}"); + Collector.Errors.Should().Be(0); + + 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_NoResolvableProduct_FallsBackToLocal() + { + // An option-mode --all filter resolves no product to scope the CDN fetch. With no --directory and + // no use_local_changelogs, the bundler should fall back to local folder sourcing rather than hit + // the CDN — preserving the pre-CDN behaviour for PR/issue-only and --all flows. + var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); + FileSystem.Directory.CreateDirectory(localDir); + await FileSystem.File.WriteAllTextAsync( + FileSystem.Path.Join(localDir, "1-local.yaml"), EntryAlpha, TestContext.Current.CancellationToken); + + var configContent = + $""" + bundle: + directory: {localDir} + """; + var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); + FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); + await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); + + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, ConfigurationContext, FileSystem, null, Fetcher(Output, handler)); + var output = OutputPath(); + + var input = new BundleChangelogsArguments { Config = configPath, Output = output, All = true, Resolve = true }; + + 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))}"); + handler.RequestedPaths.Should().BeEmpty("local fallback must not reach the CDN"); + + var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); + bundle.Should().Contain("name: 1-local.yaml"); + } + + [Fact] + public async Task RegistryFailure_FailsBundle() + { + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), + new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)), sleep: (_, _) => { }); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); + + var input = new BundleChangelogsArguments + { + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = OutputPath(), + Resolve = true + }; + + 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")); + } + + [Fact] + public async Task EntryMissingAfterRetries_FailsBundle() + { + // 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. + var handler = new StubHandler(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + return Json(RegistryJson); + if (path.EndsWith("1-alpha.yaml", StringComparison.Ordinal)) + return Yaml(EntryAlpha); + return new HttpResponseMessage(HttpStatusCode.NotFound); // 2-bravo.yaml never propagates + }); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, maxAttempts: 2, sleep: (_, _) => { }); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); + + var input = new BundleChangelogsArguments + { + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = OutputPath(), + Resolve = true + }; + + 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("2-bravo.yaml")); + } + + [Fact] + public async Task ProfileGitHubRelease_ScopesByOutputProductsAndFiltersByReleasePrs() + { + // A github_release profile resolves the product from output_products (to scope the CDN fetch) + // and the PR filter from the release body. Only the entry referenced by the release survives. + var releaseService = A.Fake(); + var outputDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString()); + FileSystem.Directory.CreateDirectory(outputDir); + + // language=yaml + var configContent = + """ + bundle: + output_directory: PLACEHOLDER + owner: elastic + profiles: + es-release: + source: github_release + repo: elasticsearch + output: "elasticsearch-{version}.yaml" + output_products: "elasticsearch {version} {lifecycle}" + """.Replace("PLACEHOLDER", outputDir); + var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); + FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); + await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); + + var releaseBody = "* Alpha by @user in https://github.com/elastic/elasticsearch/pull/100\n"; + A.CallTo(() => releaseService.FetchReleaseAsync("elastic", "elasticsearch", "9.3.0", TestContext.Current.CancellationToken)) + .Returns(new GitHubReleaseInfo { TagName = "v9.3.0", Name = "9.3.0", Body = releaseBody }); + + var service = new ChangelogBundlingService(LoggerFactory, ConfigurationContext, FileSystem, releaseService, Fetcher()); + + var input = new BundleChangelogsArguments + { + Profile = "es-release", + ProfileArgument = "9.3.0", + Config = configPath + }; + + 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))}"); + Collector.Errors.Should().Be(0); + + var outputFiles = FileSystem.Directory.GetFiles(outputDir, "*.yaml"); + outputFiles.Should().NotBeEmpty(); + var bundle = await FileSystem.File.ReadAllTextAsync(outputFiles[0], TestContext.Current.CancellationToken); + bundle.Should().Contain("Alpha"); + bundle.Should().NotContain("Bravo"); + } + + private static HttpResponseMessage Json(string body) => + new(HttpStatusCode.OK) { Content = new StringContent(body, System.Text.Encoding.UTF8, "application/json") }; + + private static HttpResponseMessage Yaml(string body) => + new(HttpStatusCode.OK) { Content = new StringContent(body, System.Text.Encoding.UTF8, "text/yaml") }; + + private sealed class StubHandler(Func responder) : HttpMessageHandler + { + public List RequestedPaths { get; } = []; + + protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) + { + RequestedPaths.Add(request.RequestUri!.AbsolutePath); + return responder(request); + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => + Task.FromResult(Send(request, cancellationToken)); + } +} diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleChangelogsTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleChangelogsTests.cs index 7e53004baa..62cfe8008c 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleChangelogsTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleChangelogsTests.cs @@ -2207,6 +2207,7 @@ public async Task BundleChangelogs_WithConfigDirectory_WhenDirectoryNotSpecified bundle: directory: "{_changelogDir.Replace("\\", "/")}" output_directory: "{outputDir.Replace("\\", "/")}" + use_local_changelogs: true """; var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, "config-dir", "changelog.yml"); @@ -3323,6 +3324,7 @@ public async Task BundleChangelogs_WithProfileMode_ConfigAtCurrentDir_LoadsSucce $$""" bundle: directory: {{Path.Join(root, "changelogs")}} + use_local_changelogs: true profiles: es-release: products: "elasticsearch {version} {lifecycle}" @@ -3382,6 +3384,7 @@ public async Task BundleChangelogs_WithProfileMode_ConfigAtDocsSubdir_LoadsSucce $$""" bundle: directory: {{Path.Join(root, "changelogs")}} + use_local_changelogs: true profiles: es-release: products: "elasticsearch {version} {lifecycle}" @@ -3432,6 +3435,7 @@ public async Task BundleChangelogs_WithProfile_UrlListFile_PrUrls_FiltersCorrect var configContent = $""" bundle: directory: {_changelogDir} + use_local_changelogs: true profiles: release: output: "bundle.yaml" @@ -3507,6 +3511,7 @@ public async Task BundleChangelogs_WithProfile_UrlListFile_IssueUrls_FiltersCorr var configContent = $""" bundle: directory: {_changelogDir} + use_local_changelogs: true profiles: release: output: "bundle.yaml" diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs index 41f1d3263a..5e8da3d8a9 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs @@ -36,6 +36,8 @@ public async Task Plan_OptionMode_ExplicitOutput_ReturnsNoNetwork() result.NeedsNetwork.Should().BeFalse(); result.NeedsGithubToken.Should().BeFalse(); result.OutputPath.Should().Be("docs/releases/my-bundle.yaml"); + // No product is resolvable in option mode without --input/output-products, so there is no CDN URL. + result.CdnUrl.Should().BeNull(); } [Fact] @@ -53,6 +55,8 @@ public async Task Plan_ReleaseVersion_ReturnsNeedsNetwork() [Fact] public async Task Plan_ProfileMode_ResolvesOutputPath() { + // A profile with a resolvable product sources entries from the CDN by default, so the plan + // reports needs_network (but not a GitHub token, since this is not a github_release profile). // language=yaml var configContent = """ @@ -75,9 +79,72 @@ public async Task Plan_ProfileMode_ResolvesOutputPath() var result = await Service.PlanBundleAsync(Collector, input, hasReleaseVersion: false, TestContext.Current.CancellationToken); result.Should().NotBeNull(); - result.NeedsNetwork.Should().BeFalse(); + result.NeedsNetwork.Should().BeTrue(); result.NeedsGithubToken.Should().BeFalse(); result.OutputPath.Should().EndWith(FileSystem.Path.Join("docs", "releases", "elasticsearch-9.2.0.yaml").OptionalWindowsReplace()); + // The bundle-PR action polls this URL for the scrubbed copy: {base}/{product}/bundle/{file}. + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/elasticsearch/bundle/elasticsearch-9.2.0.yaml"); + } + + [Fact] + public async Task Plan_ProfileMode_OutputProductsScopeCdnUrl() + { + // output_products takes precedence over products when scoping the CDN URL. + // language=yaml + var configContent = + """ + bundle: + output_directory: docs/releases + profiles: + serverless: + products: "cloud-serverless {version} *" + output_products: "cloud-serverless {version} *" + output: "serverless-{version}.yaml" + """; + var configPath = await CreateConfigAsync(configContent); + + var input = new BundleChangelogsArguments + { + Profile = "serverless", + ProfileArgument = "2026-03", + Config = configPath + }; + + var result = await Service.PlanBundleAsync(Collector, input, hasReleaseVersion: false, TestContext.Current.CancellationToken); + + result.Should().NotBeNull(); + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/cloud-serverless/bundle/serverless-2026-03.yaml"); + } + + [Fact] + public async Task Plan_ProfileMode_UseLocalChangelogs_ReturnsNoNetwork() + { + // With bundle.use_local_changelogs the entries come from the local folder, so no network is needed. + // language=yaml + var configContent = + """ + bundle: + output_directory: docs/releases + use_local_changelogs: true + profiles: + my-profile: + products: "elasticsearch {version} {lifecycle}" + output: "elasticsearch-{version}.yaml" + """; + var configPath = await CreateConfigAsync(configContent); + + var input = new BundleChangelogsArguments + { + Profile = "my-profile", + ProfileArgument = "9.2.0", + Config = configPath + }; + + var result = await Service.PlanBundleAsync(Collector, input, hasReleaseVersion: false, TestContext.Current.CancellationToken); + + result.Should().NotBeNull(); + result.NeedsNetwork.Should().BeFalse(); + result.NeedsGithubToken.Should().BeFalse(); } [Fact] diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleProfileGitHubReleaseTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleProfileGitHubReleaseTests.cs index 6efc646c74..ebf213c17e 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleProfileGitHubReleaseTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleProfileGitHubReleaseTests.cs @@ -48,6 +48,7 @@ public async Task ProfileGitHubRelease_BundlesMatchingChangelogs() """ bundle: directory: PLACEHOLDER + use_local_changelogs: true owner: elastic profiles: es-gh-release: @@ -137,6 +138,7 @@ public async Task ProfileGitHubRelease_AutoInfersVersionAndLifecycle_FromRelease """ bundle: directory: PLACEHOLDER + use_local_changelogs: true owner: elastic profiles: es-gh-release: @@ -209,6 +211,7 @@ public async Task ProfileGitHubRelease_WithNoMatchingPrs_EmitsWarning() """ bundle: directory: PLACEHOLDER + use_local_changelogs: true owner: elastic profiles: es-gh-release: @@ -289,6 +292,7 @@ public async Task ProfileGitHubRelease_Latest_CallsFetchWithLatestTag() """ bundle: directory: PLACEHOLDER + use_local_changelogs: true owner: elastic profiles: es-gh-release: @@ -468,6 +472,7 @@ public async Task ProfileGitHubRelease_InfersBetaLifecycle_FromTagSuffix() """ bundle: directory: PLACEHOLDER + use_local_changelogs: true owner: elastic profiles: es-gh-release: @@ -537,6 +542,7 @@ public async Task ProfileGitHubRelease_InfersPreviewLifecycle_FromTagSuffix() """ bundle: directory: PLACEHOLDER + use_local_changelogs: true owner: elastic profiles: es-gh-release: @@ -605,6 +611,7 @@ public async Task ProfileGitHubRelease_BundleLevelRepo_UsedWhenProfileOmitsRepo( """ bundle: directory: PLACEHOLDER + use_local_changelogs: true repo: elasticsearch owner: elastic profiles: diff --git a/tests/Elastic.Changelog.Tests/Changelogs/ChangelogConfigurationTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/ChangelogConfigurationTests.cs index 38f6222982..5ae887b627 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/ChangelogConfigurationTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/ChangelogConfigurationTests.cs @@ -1700,4 +1700,49 @@ public async Task LoadChangelogConfiguration_WithPerProductProductFiltering_Inva Collector.Errors.Should().BeGreaterThan(0); Collector.Diagnostics.Should().Contain(d => d.Message.Contains("'invalid-product' is not in the list of available products")); } + + [Fact] + public async Task LoadChangelogConfiguration_UseLocalChangelogs_DefaultsToFalse() + { + var configLoader = new ChangelogConfigurationLoader(LoggerFactory, ConfigurationContext, FileSystem); + var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); + FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); + + // language=yaml + var configContent = + """ + bundle: + directory: docs/changelog + """; + await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); + + var config = await configLoader.LoadChangelogConfiguration(Collector, configPath, TestContext.Current.CancellationToken); + + config.Should().NotBeNull(); + Collector.Errors.Should().Be(0); + config.Bundle!.UseLocalChangelogs.Should().BeFalse(); + } + + [Fact] + public async Task LoadChangelogConfiguration_UseLocalChangelogs_True_Parses() + { + var configLoader = new ChangelogConfigurationLoader(LoggerFactory, ConfigurationContext, FileSystem); + var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); + FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); + + // language=yaml + var configContent = + """ + bundle: + directory: docs/changelog + use_local_changelogs: true + """; + await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); + + var config = await configLoader.LoadChangelogConfiguration(Collector, configPath, TestContext.Current.CancellationToken); + + config.Should().NotBeNull(); + Collector.Errors.Should().Be(0); + config.Bundle!.UseLocalChangelogs.Should().BeTrue(); + } } diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs index 8ce29c5b39..d773c17d95 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs @@ -396,6 +396,37 @@ public async Task Refresh_NoTargets_WritesNothing() _puts.Should().BeEmpty(); } + [Fact] + public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() + { + var path = _mockFileSystem.Path.Join(_bundleDir, "1-feature.yaml"); + // language=yaml + _mockFileSystem.AddFile(path, new MockFileData(""" + title: Sample + type: enhancement + products: + - product: elasticsearch + target: 9.3.0 + """)); + var targets = new List { new(path, "elasticsearch/changelog/1-feature.yaml") }; + A.CallTo(() => _s3Client.GetObjectAsync(A._, A._)) + .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); + + var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken, RegistryScope.Changelog); + + result.Updated.Should().Be(1); + _puts.Should().ContainSingle(); + _puts[0].Key.Should().Be("elasticsearch/changelog/registry.json"); + + var manifest = Deserialize(_puts[0].ContentBody); + manifest.Product.Should().Be("elasticsearch"); + manifest.Bundles.Should().ContainSingle(); + manifest.Bundles[0].File.Should().Be("1-feature.yaml"); + // The changelog-entry index only enumerates files; per-entry target is not recorded. + manifest.Bundles[0].Target.Should().BeNull(); + manifest.Bundles[0].ETag.Should().NotBeNullOrEmpty(); + } + private sealed class FakeTimeProvider(DateTimeOffset now) : TimeProvider { public override DateTimeOffset GetUtcNow() => now; diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs index 221765a619..bd0091a222 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs @@ -10,11 +10,16 @@ namespace Elastic.Changelog.Tests.Uploading; public class RegistryKeyTests { [Theory] + // Bundle index: {product}/registry.json [InlineData("elasticsearch/registry.json")] [InlineData("kibana/registry.json")] [InlineData("elastic-agent/registry.json")] [InlineData("cloud_hosted/registry.json")] [InlineData("a/registry.json")] + // Changelog-entry index: {product}/changelog/registry.json + [InlineData("elasticsearch/changelog/registry.json")] + [InlineData("kibana/changelog/registry.json")] + [InlineData("cloud_hosted/changelog/registry.json")] public void IsRegistry_ValidProductKeys_ReturnsTrue(string key) => RegistryKey.IsRegistry(key).Should().BeTrue(); @@ -24,11 +29,14 @@ public void IsRegistry_ValidProductKeys_ReturnsTrue(string key) => [InlineData("/registry.json")] [InlineData("elasticsearch/bundle/registry.json")] [InlineData("elasticsearch/registry.yaml")] - [InlineData("elasticsearch/changelog/registry.json")] + [InlineData("elasticsearch/changelog/registry.yaml")] + [InlineData("elasticsearch/changelog/bundle/registry.json")] [InlineData("../registry.json")] [InlineData("elastic search/registry.json")] [InlineData("elastic.search/registry.json")] [InlineData("elastic/search/registry.json")] + [InlineData("elastic search/changelog/registry.json")] + [InlineData("elastic/search/changelog/registry.json")] public void IsRegistry_InvalidKeys_ReturnsFalse(string key) => RegistryKey.IsRegistry(key).Should().BeFalse(); } diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs new file mode 100644 index 0000000000..7c9c6c5f10 --- /dev/null +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs @@ -0,0 +1,163 @@ +// 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 System.Net; +using AwesomeAssertions; +using Elastic.Documentation.Configuration.ReleaseNotes; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Elastic.Documentation.Configuration.Tests.ReleaseNotes; + +public class CdnChangelogEntryFetcherTests +{ + // language=yaml + private const string SampleEntry = """ + title: Sample enhancement + type: enhancement + products: + - product: elasticsearch + target: 9.3.0 + """; + + private static Uri Base() => new($"https://cdn.example/{Guid.NewGuid():N}"); + + // A no-op sleeper keeps retry-exercising tests instant; a small attempt budget keeps them deterministic. + private CdnChangelogEntryFetcher CreateFetcher(StubHandler handler, int maxAttempts = 3) => + new(NullLoggerFactory.Instance, handler, maxAttempts, sleep: (_, _) => { }); + + private static (List Errors, List Warnings, Action EmitError, Action EmitWarning) Diagnostics() + { + var errors = new List(); + var warnings = new List(); + return (errors, warnings, errors.Add, warnings.Add); + } + + [Fact] + public void Fetch_HappyPath_ReturnsAllEntriesFromRegistry() + { + var handler = new StubHandler(req => + req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) + ? Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "1-a.yaml" }, { "file": "2-b.yaml" } ] }""") + : Yaml(SampleEntry)); + var (errors, warnings, emitError, emitWarning) = Diagnostics(); + + var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + + errors.Should().BeEmpty(); + warnings.Should().BeEmpty(); + entries.Select(e => e.FileName).Should().BeEquivalentTo("1-a.yaml", "2-b.yaml"); + entries.Should().OnlyContain(e => e.Content.Contains("Sample enhancement")); + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/elasticsearch/changelog/1-a.yaml", StringComparison.Ordinal)); + } + + [Fact] + public void Fetch_RegistryNotFound_EmitsErrorAndReturnsEmpty() + { + var handler = new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)); + var (errors, _, emitError, emitWarning) = Diagnostics(); + + var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + + entries.Should().BeEmpty(); + errors.Should().ContainSingle().Which.Should().Contain("registry"); + } + + [Fact] + public void Fetch_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty() + { + // A registry-listed entry that never appears on the CDN is retried, then escalated to an error + // (not skipped) so the bundle fails rather than silently dropping a release entry. + var handler = new StubHandler(req => + { + if (req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + return Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "1-a.yaml" }, { "file": "2-missing.yaml" } ] }"""); + return req.RequestUri!.AbsolutePath.EndsWith("/2-missing.yaml", StringComparison.Ordinal) + ? new HttpResponseMessage(HttpStatusCode.NotFound) + : Yaml(SampleEntry); + }); + var (errors, _, emitError, emitWarning) = Diagnostics(); + + var entries = CreateFetcher(handler, maxAttempts: 3).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + + entries.Should().BeEmpty(); + errors.Should().ContainSingle().Which.Should().Contain("2-missing.yaml"); + handler.RequestedPaths.Count(p => p.EndsWith("/2-missing.yaml", StringComparison.Ordinal)) + .Should().Be(3, "the missing entry should be attempted up to the retry budget before failing"); + } + + [Fact] + public void Fetch_EntryRecoversAfterRetry_ReturnsEntry() + { + // The common scrub/propagation race: the first GET 404s, a retry succeeds. No error, no skip. + var entryAttempts = 0; + var handler = new StubHandler(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + return Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "1-a.yaml" } ] }"""); + if (path.EndsWith("/1-a.yaml", StringComparison.Ordinal)) + return Interlocked.Increment(ref entryAttempts) == 1 + ? new HttpResponseMessage(HttpStatusCode.NotFound) + : Yaml(SampleEntry); + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + var (errors, warnings, emitError, emitWarning) = Diagnostics(); + + var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + + errors.Should().BeEmpty(); + warnings.Should().BeEmpty(); + entries.Select(e => e.FileName).Should().BeEquivalentTo("1-a.yaml"); + entryAttempts.Should().Be(2, "the first 404 should be retried and then succeed"); + } + + [Fact] + public void Fetch_SchemaVersionTooNew_EmitsError() + { + var handler = new StubHandler(_ => + Json(/*lang=json,strict*/ """{ "schema_version": 999, "product": "elasticsearch", "bundles": [] }""")); + var (errors, _, emitError, emitWarning) = Diagnostics(); + + var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + + entries.Should().BeEmpty(); + errors.Should().ContainSingle().Which.Should().Contain("schema version"); + } + + [Fact] + public void Fetch_UnsafeFileName_EmitsWarningAndSkips() + { + var handler = new StubHandler(req => + req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) + ? Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "../escape.yaml" }, { "file": "ok.yaml" } ] }""") + : Yaml(SampleEntry)); + var (errors, warnings, emitError, emitWarning) = Diagnostics(); + + var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + + errors.Should().BeEmpty(); + entries.Select(e => e.FileName).Should().BeEquivalentTo("ok.yaml"); + warnings.Should().ContainSingle().Which.Should().Contain("escape.yaml"); + } + + private static HttpResponseMessage Json(string body) => + new(HttpStatusCode.OK) { Content = new StringContent(body, System.Text.Encoding.UTF8, "application/json") }; + + private static HttpResponseMessage Yaml(string body) => + new(HttpStatusCode.OK) { Content = new StringContent(body, System.Text.Encoding.UTF8, "text/yaml") }; + + private sealed class StubHandler(Func responder) : HttpMessageHandler + { + public List RequestedPaths { get; } = []; + + protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) + { + RequestedPaths.Add(request.RequestUri!.AbsolutePath); + return responder(request); + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => + Task.FromResult(Send(request, cancellationToken)); + } +} From 7e3087d87d2869c710caaad7657d99a342a1d249 Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Fri, 19 Jun 2026 00:13:38 -0300 Subject: [PATCH 2/2] changelog: gate CDN bundle sourcing on docset.yml release_notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make `changelog bundle` source entries from the CDN only when every in-scope product is declared under `release_notes` in docset.yml, matching the directive's opt-in model. Undeclared products — and `use_local_changelogs` or `--directory` — fall back to local sourcing. The same declared-gate drives the `--plan` `needs_network` output so the planning step and bundle run agree. Also make CdnChangelogEntryFetcher IDisposable and fully async (shared HttpClient in production, owned client only for injected test handlers, async retry backoff), addressing the HttpClient review feedback. Updates bundle configuration/registry docs and tests. Co-authored-by: Cursor --- docs/contribute/configure-changelogs-ref.md | 18 +++ docs/development/changelog-bundle-registry.md | 22 +++ .../ReleaseNotes/CdnChangelogEntryFetcher.cs | 111 ++++++++++---- .../Bundling/ChangelogBundlingService.cs | 139 ++++++++++++++---- .../Changelogs/BundleCdnSourcingTests.cs | 51 ++++++- .../Changelogs/BundlePlanTests.cs | 35 ++++- .../Changelogs/ChangelogTestBase.cs | 13 ++ .../CdnChangelogEntryFetcherTests.cs | 45 +++--- 8 files changed, 347 insertions(+), 87 deletions(-) diff --git a/docs/contribute/configure-changelogs-ref.md b/docs/contribute/configure-changelogs-ref.md index 5736771cb9..edc321364d 100644 --- a/docs/contribute/configure-changelogs-ref.md +++ b/docs/contribute/configure-changelogs-ref.md @@ -53,6 +53,7 @@ These settings are relevant to one or all of the `changelog bundle`, `changelog | `bundle.release_dates` | When `true`, bundles include a `release-date` field (default: true). | | `bundle.repo` | Default GitHub repository name (for example, `elasticsearch`). Used by the `{changelog}` directive to generate correct PR and issue links. Only needed when the product ID doesn't match the GitHub repository name. | | `bundle.resolve` | When `true`, changelog contents are copied into bundle (default: `true`). | +| `bundle.use_local_changelogs` | When `true`, always source entries from the local folder and never from the CDN (default: `false`). Refer to [Entry sourcing](#bundle-entry-sourcing). | ::: @@ -63,6 +64,23 @@ When `bundle.link_allow_repos` is omitted, no link filtering occurs. - For public repos, add your `owner/repo` to the list at a minimum. ::: +### Entry sourcing [bundle-entry-sourcing] + +`changelog bundle` reads the individual changelog entries it aggregates either from the local folder or from the public CDN. CDN sourcing is **opt-in per product** (declared-gate): an entry is pulled from the CDN only when its product is declared under `release_notes` in the docset's `docset.yml`. + +```yaml +# docset.yml +release_notes: + - product: elasticsearch +``` + +Sourcing is decided per run: + +- **Local folder (default).** Used when no product in scope is declared under `release_notes`, when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when the filter resolves no concrete product (for example, `--all` or PR/issue-only filters). The folder must contain the changelog files. +- **CDN.** Used only when every product in scope is declared under `release_notes` and none of the local-sourcing conditions above apply. The product must also exist in [products.yml](https://github.com/elastic/docs-builder/blob/main/config/products.yml) with the `release-notes` feature enabled. + +The product ID under `release_notes` matches the product format described in [](/cli/changelog/bundle.md#product-format). This is the same declaration the `{changelog}` directive's `:cdn:` mode consumes, so a repository that opts into CDN-sourced bundling and CDN-rendered release notes declares each product once. + ### Bundle descriptions [bundle-descriptions] You can add introductory text to bundles using the `description` field. This text appears at the top of rendered changelogs, after the release heading but before the entry sections. diff --git a/docs/development/changelog-bundle-registry.md b/docs/development/changelog-bundle-registry.md index 790e644f87..26b6b67c45 100644 --- a/docs/development/changelog-bundle-registry.md +++ b/docs/development/changelog-bundle-registry.md @@ -166,6 +166,28 @@ build that includes this feature. Consumers must therefore treat a missing bundle as non-fatal (skip + warn), not an error. +## `changelog bundle` entry sourcing (declared-gate) + +The `changelog bundle` command aggregates individual changelog **entries**. It can read those +entries from the local folder or fetch a product's published entries from the CDN +(`{product}/changelog/registry.json` → `{product}/changelog/{file}`, via `CdnChangelogEntryFetcher`). + +CDN entry sourcing is **opt-in per product** (a *declared-gate*): a product's entries are pulled from +the CDN only when that product is declared under `release_notes` in the repo's `docset.yml` — the same +declaration the directive consumes. The decision is made per run by `ChangelogBundlingService`: + +- **Local folder** when `bundle.use_local_changelogs: true`, when `--directory` is passed, when no + concrete product is in scope (for example `--all` or PR/issue-only filters), or when any in-scope + product is **not** declared under `release_notes`. +- **CDN** only when every in-scope product is declared. The declared set is read with + `DocumentationSetFile.LoadMetadata` from the known docset locations (repo root or `docs/`). + +The same gate drives the `--plan` `needs_network` output, so a planning step and the actual bundle +run agree on whether the Docker bundle needs network access. The registry-fetch is fail-fast and an +entry still missing after its retry budget fails the bundle (an incomplete release would otherwise +ship silently). `CdnChangelogEntryFetcher` reuses a shared `HttpClient` in production and disposes an +owned client only when a test injects a handler, mirroring `CdnChangelogFetcher`. + ## Consumer: `{changelog}` directive `cdn:` mode (implemented) ### Syntax diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs index f51833945a..30fe0fd8e4 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs @@ -2,6 +2,7 @@ // 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 System.Net; using System.Text.Json; using Microsoft.Extensions.Logging; @@ -30,11 +31,7 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; /// silently shipping an incomplete release bundle is worse than failing the run. /// /// -public sealed class CdnChangelogEntryFetcher( - ILoggerFactory logFactory, - HttpMessageHandler? handler = null, - int maxAttempts = CdnChangelogEntryFetcher.DefaultMaxAttempts, - Action? sleep = null) +public sealed class CdnChangelogEntryFetcher : IDisposable { private const int SupportedSchemaVersion = 1; @@ -43,10 +40,56 @@ public sealed class CdnChangelogEntryFetcher( private const int BaseRetryDelayMs = 500; private const int MaxRetryDelayMs = 2000; - private readonly ILogger _logger = logFactory.CreateLogger(); - private readonly HttpClient _httpClient = handler is null ? new HttpClient() : new HttpClient(handler, disposeHandler: false); - private readonly int _maxAttempts = maxAttempts < 1 ? DefaultMaxAttempts : maxAttempts; - private readonly Action _sleep = sleep ?? DefaultSleep; + /// + /// Bounds an individual registry/entry HTTP request so a stalled CDN connection cannot hang a bundle run. + /// + private static readonly TimeSpan FetchTimeout = TimeSpan.FromSeconds(30); + + /// + /// Process-wide client shared by every fetcher built for the production (no injected handler) path. + /// is thread-safe and intended to be long-lived; a single static instance avoids + /// leaking a socket handle per fetch, and + /// bounds DNS staleness. It is intentionally never disposed — it lives for the lifetime of the process. + /// + private static readonly HttpClient SharedHttpClient = new( + new SocketsHttpHandler + { + AutomaticDecompression = DecompressionMethods.All, + PooledConnectionLifetime = TimeSpan.FromMinutes(5) + }) + { Timeout = FetchTimeout }; + + private readonly ILogger _logger; + private readonly HttpClient _httpClient; + private readonly int _maxAttempts; + private readonly Func _sleep; + + /// + /// Non-null only when a caller injects its own (tests): in that case we + /// own a per-instance client and must dispose it. On the production path points + /// at , which is never disposed. + /// + private readonly HttpClient? _ownedHttpClient; + + public CdnChangelogEntryFetcher( + ILoggerFactory logFactory, + HttpMessageHandler? handler = null, + int maxAttempts = DefaultMaxAttempts, + Func? sleep = null) + { + _logger = logFactory.CreateLogger(); + _maxAttempts = maxAttempts < 1 ? DefaultMaxAttempts : maxAttempts; + _sleep = sleep ?? DefaultSleepAsync; + + if (handler is null) + _httpClient = SharedHttpClient; + else + { + // disposeHandler: false — the injected handler is owned by the caller (tests), not by us. + _ownedHttpClient = new HttpClient(handler, disposeHandler: false) { Timeout = FetchTimeout }; + _httpClient = _ownedHttpClient; + } + } /// /// Downloads the changelog entries for from the CDN at @@ -54,7 +97,7 @@ public sealed class CdnChangelogEntryFetcher( /// be read or when a registry-listed entry cannot be fetched within the retry budget. Entries are /// returned in registry order; the caller owns filtering and de-duplication. /// - public IReadOnlyList Fetch( + public async Task> FetchAsync( Uri baseUri, string product, Action emitError, @@ -66,7 +109,7 @@ public IReadOnlyList Fetch( ChangelogRegistry? registry; try { - registry = FetchRegistry(registryUri, ctx); + registry = await FetchRegistryAsync(registryUri, ctx).ConfigureAwait(false); } catch (Exception ex) when (ex is not OperationCanceledException) { @@ -100,7 +143,8 @@ public IReadOnlyList Fetch( } var entryUri = Combine(baseUri, product, "changelog", fileName); - if (TryFetchEntry(entryUri, fileName, product, ctx, out var content, out var lastError)) + var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, product, ctx).ConfigureAwait(false); + if (fetched) { entries.Add(new CdnChangelogEntry(fileName, content)); continue; @@ -124,20 +168,19 @@ public IReadOnlyList Fetch( /// up to times with exponential backoff. Retry requests are cache-busted /// so a CloudFront-cached 404 cannot pin the result for the whole window. /// - private bool TryFetchEntry(Uri uri, string fileName, string product, Cancel ctx, out string content, out string? lastError) + private async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string product, Cancel ctx) { - content = string.Empty; - lastError = null; + string? lastError = null; for (var attempt = 1; attempt <= _maxAttempts; attempt++) { ctx.ThrowIfCancellationRequested(); try { - content = FetchText(uri, attempt, ctx); + var content = await FetchTextAsync(uri, attempt, ctx).ConfigureAwait(false); if (attempt > 1) _logger.LogInformation("Fetched changelog entry '{File}' for {Product} on attempt {Attempt}/{Max}", fileName, product, attempt, _maxAttempts); - return true; + return (true, content, null); } catch (Exception ex) when (ex is not OperationCanceledException) { @@ -149,24 +192,24 @@ private bool TryFetchEntry(Uri uri, string fileName, string product, Cancel ctx, _logger.LogDebug( "Changelog entry '{File}' for {Product} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", fileName, product, attempt, _maxAttempts, ex.Message, delay); - _sleep(delay, ctx); + await _sleep(delay, ctx).ConfigureAwait(false); } } - return false; + return (false, string.Empty, lastError); } - private ChangelogRegistry? FetchRegistry(Uri registryUri, Cancel ctx) + private async Task FetchRegistryAsync(Uri registryUri, Cancel ctx) { _logger.LogInformation("Fetching changelog entry registry {RegistryUri}", registryUri); using var request = new HttpRequestMessage(HttpMethod.Get, registryUri); - using var response = _httpClient.Send(request, ctx); + using var response = await _httpClient.SendAsync(request, ctx).ConfigureAwait(false); _ = response.EnsureSuccessStatusCode(); - using var stream = response.Content.ReadAsStream(ctx); - return JsonSerializer.Deserialize(stream, ChangelogRegistryJsonContext.Default.ChangelogRegistry); + await using var stream = await response.Content.ReadAsStreamAsync(ctx).ConfigureAwait(false); + return await JsonSerializer.DeserializeAsync(stream, ChangelogRegistryJsonContext.Default.ChangelogRegistry, ctx).ConfigureAwait(false); } - private string FetchText(Uri uri, int attempt, Cancel ctx) + private async Task FetchTextAsync(Uri uri, int attempt, Cancel ctx) { // Only bust the cache on retries: the first hit should use the CDN cache normally (the common, // already-propagated case); retries explicitly want to bypass any cached 404. @@ -174,11 +217,9 @@ private string FetchText(Uri uri, int attempt, Cancel ctx) using var request = new HttpRequestMessage(HttpMethod.Get, requestUri); if (attempt > 1) _ = request.Headers.TryAddWithoutValidation("Cache-Control", "no-cache"); - using var response = _httpClient.Send(request, ctx); + using var response = await _httpClient.SendAsync(request, ctx).ConfigureAwait(false); _ = response.EnsureSuccessStatusCode(); - using var stream = response.Content.ReadAsStream(ctx); - using var reader = new StreamReader(stream); - return reader.ReadToEnd(); + return await response.Content.ReadAsStringAsync(ctx).ConfigureAwait(false); } private static TimeSpan RetryDelay(int attempt) @@ -188,10 +229,10 @@ private static TimeSpan RetryDelay(int attempt) return TimeSpan.FromMilliseconds(ms); } - private static void DefaultSleep(TimeSpan delay, Cancel ctx) + private static async Task DefaultSleepAsync(TimeSpan delay, Cancel ctx) { if (delay > TimeSpan.Zero) - _ = ctx.WaitHandle.WaitOne(delay); + await Task.Delay(delay, ctx).ConfigureAwait(false); } private static Uri WithCacheBuster(Uri uri) @@ -215,4 +256,14 @@ private static bool IsSafeFileName(string fileName) => !fileName.Contains('/', StringComparison.Ordinal) && !fileName.Contains('\\', StringComparison.Ordinal) && fileName is not ("." or ".."); + + /// + /// Disposes the per-instance created for an injected handler. The shared + /// production client () is process-lived and intentionally not disposed. + /// + public void Dispose() + { + _ownedHttpClient?.Dispose(); + GC.SuppressFinalize(this); + } } diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index ed7c8eed88..329703a437 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -2,6 +2,7 @@ // 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 System.Collections.Frozen; using System.IO.Abstractions; using System.Security.Cryptography; using System.Text; @@ -14,6 +15,7 @@ using Elastic.Documentation.Configuration.Assembler; using Elastic.Documentation.Configuration.Changelog; using Elastic.Documentation.Configuration.ReleaseNotes; +using Elastic.Documentation.Configuration.Toc; using Elastic.Documentation.Diagnostics; using Elastic.Documentation.Extensions; using Elastic.Documentation.ReleaseNotes; @@ -151,6 +153,8 @@ public partial class ChangelogBundlingService( /// private static readonly UTF8Encoding Utf8NoBom = new(encoderShouldEmitUTF8Identifier: false); + private static readonly string[] DocsetFileNames = ["docset.yml", "_docset.yml"]; + [GeneratedRegex(@"(\s+)version:", RegexOptions.Multiline)] internal static partial Regex VersionToTargetRegex(); @@ -210,14 +214,15 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // Apply config defaults if available input = ApplyConfigDefaults(input, config); - // Decide where the individual changelog entries come from. Default is the public CDN, scoped - // to the bundle's products. Fall back to local folder sourcing when the user opted in - // (bundle.use_local_changelogs), passed an explicit --directory, or when no concrete product - // can be resolved to scope the per-product CDN fetch (e.g. an option-mode PR/issue-only - // filter). This keeps the run in lockstep with PlanBundleAsync's needs_network decision. + // Decide where the individual changelog entries come from. CDN sourcing is opt-in via the + // repo's docset.yml release_notes (declared-gate): a product's entries are pulled from the + // public CDN only when it is declared there. Otherwise — or when the user forces local + // sourcing (bundle.use_local_changelogs / --directory) or no concrete product is in scope — + // fall back to the local folder. This keeps the run in lockstep with PlanBundleAsync's + // needs_network decision. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var cdnProducts = ResolveCdnProducts(input); - var useCdn = !useLocalChangelogs && !explicitDirectory && cdnProducts.Count > 0; + var useCdn = ShouldSourceFromCdn(cdnProducts, useLocalChangelogs, explicitDirectory); // Validate input. In CDN mode the local input directory is not read, so its existence // is not required. @@ -265,7 +270,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle ChangelogMatchResult matchResult; if (useCdn) { - var contents = FetchCdnEntries(collector, cdnProducts, ctx); + var contents = await FetchCdnEntriesAsync(collector, cdnProducts, ctx); if (contents == null) return false; matchResult = entryMatcher.MatchChangelogContents(collector, contents, filterCriteria, ctx); @@ -644,16 +649,14 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } } - // CDN entry sourcing (the default) needs network access for the Docker bundle run. It is active - // unless the user opted into local sourcing (bundle.use_local_changelogs) or passed --directory, - // and only when a product can be resolved to scope the fetch. + // CDN entry sourcing needs network access for the Docker bundle run. Mirror the run-mode + // declared-gate: active only when every in-scope product is declared in docset.yml release_notes + // and the user has not forced local sourcing. Products are resolved from the profile patterns + // (not yet materialized in plan mode) unioned with any explicit --input/--output-products. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var explicitDirectory = !string.IsNullOrWhiteSpace(input.Directory); - var cdnProductsResolvable = !string.IsNullOrWhiteSpace(input.Profile) - ? profileDef is not null && - (!string.IsNullOrWhiteSpace(profileDef.Products) || !string.IsNullOrWhiteSpace(profileDef.OutputProducts)) - : input.InputProducts is { Count: > 0 } || input.OutputProducts is { Count: > 0 }; - if (!useLocalChangelogs && !explicitDirectory && cdnProductsResolvable) + var cdnProducts = ResolveCdnProducts(input, profileDef); + if (ShouldSourceFromCdn(cdnProducts, useLocalChangelogs, explicitDirectory)) needsNetwork = true; // Resolve output path — mirrors the logic in ProcessProfile + ApplyConfigDefaults. @@ -683,10 +686,8 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } /// - /// Builds the public CDN URL where the scrubbed bundle will be served once uploaded: - /// {base}/{product}/bundle/{file}. The bundle-PR action polls this to fetch the scrubbed copy - /// rather than committing the raw, locally-generated file. Returns null when the product, output file - /// name, or CDN base cannot be resolved. + /// Public CDN URL of the scrubbed bundle ({base}/{product}/bundle/{file}), polled by the + /// bundle-PR action. Null when the product, output file name, or CDN base cannot be resolved. /// private string? ResolveCdnBundleUrl(BundleProfile? profileDef, BundleChangelogsArguments input, string? outputPath) { @@ -709,9 +710,8 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } /// - /// The first concrete (non-wildcard) product that scopes the bundle, used to build its CDN URL. Taken - /// from the profile's output_products/products pattern (first whitespace-delimited token - /// of the first comma-group) or, in option mode, the first explicit output/input product argument. + /// The first concrete (non-wildcard) product that scopes the bundle, used to build its CDN URL. + /// From the profile output_products/products pattern, else the first explicit product argument. /// private static string? ResolvePrimaryProduct(BundleProfile? profileDef, BundleChangelogsArguments input) { @@ -740,10 +740,10 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments /// /// Downloads the in-scope changelog entries from the public CDN, scoped to the bundle's products. - /// Returns null (after emitting an error) when no product can be resolved or a per-product - /// registry cannot be fetched; an individual entry that is not yet public is skipped with a warning. + /// Returns null (after emitting an error) on any fatal fetch failure; an entry not yet public is + /// skipped with a warning. /// - private IReadOnlyList<(string FileName, string Content)>? FetchCdnEntries( + private async Task?> FetchCdnEntriesAsync( IDiagnosticsCollector collector, IReadOnlyList products, Cancel ctx) @@ -751,9 +751,9 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments if (products.Count == 0) { collector.EmitError(string.Empty, - "Sourcing changelog entries from the CDN requires a resolvable product. Use a profile or " + - "--input-products with a concrete product, set bundle.use_local_changelogs: true in " + - "changelog.yml, or pass --directory to bundle local changelog files."); + "Sourcing changelog entries from the CDN requires a resolvable product declared under " + + "release_notes in docset.yml. Declare the product there, or set bundle.use_local_changelogs: " + + "true in changelog.yml / pass --directory to bundle local changelog files."); return null; } @@ -769,7 +769,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments var fatalFailure = false; foreach (var product in products) { - var entries = _entryFetcher.Fetch( + var entries = await _entryFetcher.FetchAsync( baseUri, product, msg => { fatalFailure = true; collector.EmitError(string.Empty, msg); }, @@ -793,12 +793,16 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } /// - /// The distinct, concrete product IDs that scope a CDN-sourced bundle, taken from the resolved - /// input products (profile / --input-products) and any explicit output products. Wildcards are excluded. + /// The distinct, concrete product IDs that scope a CDN-sourced bundle. In run mode the input is already + /// materialized, so the ids come from input.InputProducts/OutputProducts. In plan mode pass + /// so the ids can also be read from the profile's (not yet materialized) + /// products/output_products patterns. Wildcards are excluded. /// - private static IReadOnlyList ResolveCdnProducts(BundleChangelogsArguments input) + private static IReadOnlyList ResolveCdnProducts(BundleChangelogsArguments input, BundleProfile? profileDef = null) { var ids = new List(); + AppendConcreteProductsFromPattern(ids, profileDef?.OutputProducts); + AppendConcreteProductsFromPattern(ids, profileDef?.Products); AppendConcreteProducts(ids, input.InputProducts); AppendConcreteProducts(ids, input.OutputProducts); return ids.Distinct(StringComparer.Ordinal).ToList(); @@ -815,6 +819,77 @@ private static void AppendConcreteProducts(List ids, IReadOnlyList ids, string? pattern) + { + if (string.IsNullOrWhiteSpace(pattern)) + return; + foreach (var group in pattern.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) + { + var id = group.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault(); + if (!string.IsNullOrWhiteSpace(id) && id != "*") + ids.Add(id); + } + } + + /// + /// Declared-gate for CDN entry sourcing: the public CDN is used only when every in-scope product is + /// declared under release_notes in the repo's docset.yml. Returns false (local sourcing) when + /// the user forced local sourcing (bundle.use_local_changelogs or an explicit --directory), + /// no concrete product is in scope, or any in-scope product is undeclared. + /// + private bool ShouldSourceFromCdn(IReadOnlyList cdnProducts, bool useLocalChangelogs, bool explicitDirectory) + { + if (useLocalChangelogs || explicitDirectory || cdnProducts.Count == 0) + return false; + var declared = LoadDeclaredReleaseNotesProducts(); + return cdnProducts.All(declared.Contains); + } + + /// + /// Product ids declared under release_notes in the repo's docset.yml, normalized (underscores → + /// hyphens). Returns an empty set when no docset.yml is found in the known locations or it declares none. + /// + private IReadOnlySet LoadDeclaredReleaseNotesProducts() + { + var configFile = FindDocsetFile(); + if (configFile is null) + return FrozenSet.Empty; + try + { + var docSet = DocumentationSetFile.LoadMetadata(configFile); + return docSet.ReleaseNotes + .Select(r => r.Product?.Trim().Replace('_', '-')) + .Where(p => !string.IsNullOrEmpty(p)) + .Select(p => p!) + .ToFrozenSet(StringComparer.Ordinal); + } + catch (Exception ex) + { + _logger.LogDebug("Could not read release_notes from docset.yml for CDN gating: {Error}", ex.Message); + return FrozenSet.Empty; + } + } + + // Mirrors Paths.GetDocsetPathFromKnownLocations (repo root or docs/, docset.yml or _docset.yml) without + // taking a dependency on the tooling layer from this service. + private IFileInfo? FindDocsetFile() + { + var cwd = _fileSystem.Directory.GetCurrentDirectory(); + string[] folders = [cwd, _fileSystem.Path.Join(cwd, "docs")]; + foreach (var folder in folders) + { + foreach (var name in DocsetFileNames) + { + var candidate = _fileSystem.Path.Join(folder, name); + if (_fileSystem.File.Exists(candidate)) + return _fileSystem.FileInfo.New(candidate); + } + } + return null; + } + private bool ValidateInput(IDiagnosticsCollector collector, BundleChangelogsArguments input, bool requireDirectoryExists) { if (string.IsNullOrWhiteSpace(input.Directory)) diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs index faea3af32e..090053b1ad 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs @@ -61,7 +61,7 @@ public class BundleCdnSourcingTests(ITestOutputHelper output) : ChangelogTestBas // No-op sleeper so any entry retry stays instant in tests. private static CdnChangelogEntryFetcher Fetcher(ITestOutputHelper output, StubHandler handler) => - new(new TestLoggerFactory(output), handler, sleep: (_, _) => { }); + new(new TestLoggerFactory(output), handler, sleep: (_, _) => Task.CompletedTask); private CdnChangelogEntryFetcher Fetcher() => Fetcher(Output, RegistryHandler()); @@ -71,6 +71,7 @@ private string OutputPath() => [Fact] public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() { + DeclareReleaseNotesProducts("elasticsearch"); var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher()); var output = OutputPath(); @@ -127,11 +128,53 @@ await FileSystem.File.WriteAllTextAsync( bundle.Should().Contain("name: 1-local.yaml"); } + [Fact] + public async Task OptionMode_UndeclaredProduct_FallsBackToLocal() + { + // The declared-gate: a product not listed under docset.yml release_notes is never sourced from the + // CDN, even with a resolvable product filter and no --directory. The bundler falls back to local + // folder sourcing instead. Here elasticsearch is intentionally NOT declared. + var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); + FileSystem.Directory.CreateDirectory(localDir); + await FileSystem.File.WriteAllTextAsync( + FileSystem.Path.Join(localDir, "1-local.yaml"), EntryAlpha, TestContext.Current.CancellationToken); + + var configContent = + $""" + bundle: + directory: {localDir} + """; + var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); + FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); + await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); + + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, ConfigurationContext, FileSystem, null, Fetcher(Output, handler)); + var output = OutputPath(); + + var input = new BundleChangelogsArguments + { + Config = configPath, + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = output, + Resolve = true + }; + + 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))}"); + handler.RequestedPaths.Should().BeEmpty("an undeclared product must not reach the CDN"); + + var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); + bundle.Should().Contain("name: 1-local.yaml"); + } + [Fact] public async Task RegistryFailure_FailsBundle() { + DeclareReleaseNotesProducts("elasticsearch"); var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), - new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)), sleep: (_, _) => { }); + new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)), sleep: (_, _) => Task.CompletedTask); var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); var input = new BundleChangelogsArguments @@ -161,7 +204,8 @@ public async Task EntryMissingAfterRetries_FailsBundle() return Yaml(EntryAlpha); return new HttpResponseMessage(HttpStatusCode.NotFound); // 2-bravo.yaml never propagates }); - var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, maxAttempts: 2, sleep: (_, _) => { }); + var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, maxAttempts: 2, sleep: (_, _) => Task.CompletedTask); + DeclareReleaseNotesProducts("elasticsearch"); var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); var input = new BundleChangelogsArguments @@ -182,6 +226,7 @@ public async Task ProfileGitHubRelease_ScopesByOutputProductsAndFiltersByRelease { // A github_release profile resolves the product from output_products (to scope the CDN fetch) // and the PR filter from the release body. Only the entry referenced by the release survives. + DeclareReleaseNotesProducts("elasticsearch"); var releaseService = A.Fake(); var outputDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString()); FileSystem.Directory.CreateDirectory(outputDir); diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs index 5e8da3d8a9..ff3b56625a 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs @@ -55,8 +55,9 @@ public async Task Plan_ReleaseVersion_ReturnsNeedsNetwork() [Fact] public async Task Plan_ProfileMode_ResolvesOutputPath() { - // A profile with a resolvable product sources entries from the CDN by default, so the plan + // A profile whose product is declared under release_notes sources entries from the CDN, so the plan // reports needs_network (but not a GitHub token, since this is not a github_release profile). + DeclareReleaseNotesProducts("elasticsearch"); // language=yaml var configContent = """ @@ -86,6 +87,38 @@ public async Task Plan_ProfileMode_ResolvesOutputPath() result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/elasticsearch/bundle/elasticsearch-9.2.0.yaml"); } + [Fact] + public async Task Plan_ProfileMode_UndeclaredProduct_ReturnsNoNetwork() + { + // Declared-gate at plan time: with the product absent from docset.yml release_notes, the bundle + // run will source locally, so the plan reports no network. The CDN URL is still resolved because + // it only describes where a (possibly local) bundle would be published, independent of the gate. + // language=yaml + var configContent = + """ + bundle: + output_directory: docs/releases + profiles: + my-profile: + products: "elasticsearch {version} {lifecycle}" + output: "elasticsearch-{version}.yaml" + """; + var configPath = await CreateConfigAsync(configContent); + + var input = new BundleChangelogsArguments + { + Profile = "my-profile", + ProfileArgument = "9.2.0", + Config = configPath + }; + + var result = await Service.PlanBundleAsync(Collector, input, hasReleaseVersion: false, TestContext.Current.CancellationToken); + + result.Should().NotBeNull(); + result.NeedsNetwork.Should().BeFalse(); + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/elasticsearch/bundle/elasticsearch-9.2.0.yaml"); + } + [Fact] public async Task Plan_ProfileMode_OutputProductsScopeCdnUrl() { diff --git a/tests/Elastic.Changelog.Tests/Changelogs/ChangelogTestBase.cs b/tests/Elastic.Changelog.Tests/Changelogs/ChangelogTestBase.cs index 832d20ab2c..7dd17b290d 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/ChangelogTestBase.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/ChangelogTestBase.cs @@ -112,6 +112,19 @@ protected ChangelogTestBase(ITestOutputHelper output) }; } + /// + /// Seeds a minimal docset.yml at the working-directory root declaring the given products under + /// release_notes. The changelog bundle declared-gate sources a product's entries from + /// the CDN only when it is declared here. + /// + protected void DeclareReleaseNotesProducts(params string[] productIds) + { + var lines = new List { "release_notes:" }; + lines.AddRange(productIds.Select(id => $" - product: {id}")); + var path = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, "docset.yml"); + FileSystem.File.WriteAllText(path, string.Join("\n", lines) + "\n"); + } + public void Dispose() { LoggerFactory.Dispose(); diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs index 7c9c6c5f10..9839e7a359 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs @@ -20,11 +20,11 @@ public class CdnChangelogEntryFetcherTests target: 9.3.0 """; - private static Uri Base() => new($"https://cdn.example/{Guid.NewGuid():N}"); + private static readonly Uri BaseUri = new("https://cdn.example"); - // A no-op sleeper keeps retry-exercising tests instant; a small attempt budget keeps them deterministic. - private CdnChangelogEntryFetcher CreateFetcher(StubHandler handler, int maxAttempts = 3) => - new(NullLoggerFactory.Instance, handler, maxAttempts, sleep: (_, _) => { }); + // A no-op async sleeper keeps retry-exercising tests instant; a small attempt budget keeps them deterministic. + private static CdnChangelogEntryFetcher CreateFetcher(StubHandler handler, int maxAttempts = 3) => + new(NullLoggerFactory.Instance, handler, maxAttempts, sleep: (_, _) => Task.CompletedTask); private static (List Errors, List Warnings, Action EmitError, Action EmitWarning) Diagnostics() { @@ -34,7 +34,7 @@ private static (List Errors, List Warnings, Action EmitE } [Fact] - public void Fetch_HappyPath_ReturnsAllEntriesFromRegistry() + public async Task FetchAsync_HappyPath_ReturnsAllEntriesFromRegistry() { var handler = new StubHandler(req => req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) @@ -42,7 +42,8 @@ public void Fetch_HappyPath_ReturnsAllEntriesFromRegistry() : Yaml(SampleEntry)); var (errors, warnings, emitError, emitWarning) = Diagnostics(); - var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); warnings.Should().BeEmpty(); @@ -52,19 +53,20 @@ public void Fetch_HappyPath_ReturnsAllEntriesFromRegistry() } [Fact] - public void Fetch_RegistryNotFound_EmitsErrorAndReturnsEmpty() + public async Task FetchAsync_RegistryNotFound_EmitsErrorAndReturnsEmpty() { var handler = new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)); var (errors, _, emitError, emitWarning) = Diagnostics(); - var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("registry"); } [Fact] - public void Fetch_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty() + public async Task FetchAsync_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty() { // A registry-listed entry that never appears on the CDN is retried, then escalated to an error // (not skipped) so the bundle fails rather than silently dropping a release entry. @@ -78,7 +80,8 @@ public void Fetch_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty() }); var (errors, _, emitError, emitWarning) = Diagnostics(); - var entries = CreateFetcher(handler, maxAttempts: 3).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + using var fetcher = CreateFetcher(handler, maxAttempts: 3); + var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("2-missing.yaml"); @@ -87,7 +90,7 @@ public void Fetch_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty() } [Fact] - public void Fetch_EntryRecoversAfterRetry_ReturnsEntry() + public async Task FetchAsync_EntryRecoversAfterRetry_ReturnsEntry() { // The common scrub/propagation race: the first GET 404s, a retry succeeds. No error, no skip. var entryAttempts = 0; @@ -104,7 +107,8 @@ public void Fetch_EntryRecoversAfterRetry_ReturnsEntry() }); var (errors, warnings, emitError, emitWarning) = Diagnostics(); - var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); warnings.Should().BeEmpty(); @@ -113,20 +117,21 @@ public void Fetch_EntryRecoversAfterRetry_ReturnsEntry() } [Fact] - public void Fetch_SchemaVersionTooNew_EmitsError() + public async Task FetchAsync_SchemaVersionTooNew_EmitsError() { var handler = new StubHandler(_ => Json(/*lang=json,strict*/ """{ "schema_version": 999, "product": "elasticsearch", "bundles": [] }""")); var (errors, _, emitError, emitWarning) = Diagnostics(); - var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("schema version"); } [Fact] - public void Fetch_UnsafeFileName_EmitsWarningAndSkips() + public async Task FetchAsync_UnsafeFileName_EmitsWarningAndSkips() { var handler = new StubHandler(req => req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) @@ -134,7 +139,8 @@ public void Fetch_UnsafeFileName_EmitsWarningAndSkips() : Yaml(SampleEntry)); var (errors, warnings, emitError, emitWarning) = Diagnostics(); - var entries = CreateFetcher(handler).Fetch(Base(), "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); entries.Select(e => e.FileName).Should().BeEquivalentTo("ok.yaml"); @@ -151,13 +157,10 @@ private sealed class StubHandler(Func r { public List RequestedPaths { get; } = []; - protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { RequestedPaths.Add(request.RequestUri!.AbsolutePath); - return responder(request); + return Task.FromResult(responder(request)); } - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => - Task.FromResult(Send(request, cancellationToken)); } }