Skip to content

Commit 332a2af

Browse files
cotticursoragent
andcommitted
changelog: gate CDN bundle sourcing on docset.yml release_notes
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 <cursoragent@cursor.com>
1 parent aaf2156 commit 332a2af

8 files changed

Lines changed: 347 additions & 87 deletions

File tree

docs/contribute/configure-changelogs-ref.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ These settings are relevant to one or all of the `changelog bundle`, `changelog
5353
| `bundle.release_dates` | When `true`, bundles include a `release-date` field (default: true). |
5454
| `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. |
5555
| `bundle.resolve` | When `true`, changelog contents are copied into bundle (default: `true`). |
56+
| `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). |
5657

5758
:::
5859

@@ -63,6 +64,23 @@ When `bundle.link_allow_repos` is omitted, no link filtering occurs.
6364
- For public repos, add your `owner/repo` to the list at a minimum.
6465
:::
6566

67+
### Entry sourcing [bundle-entry-sourcing]
68+
69+
`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`.
70+
71+
```yaml
72+
# docset.yml
73+
release_notes:
74+
- product: elasticsearch
75+
```
76+
77+
Sourcing is decided per run:
78+
79+
- **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.
80+
- **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.
81+
82+
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.
83+
6684
### Bundle descriptions [bundle-descriptions]
6785

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

docs/development/changelog-bundle-registry.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,28 @@ build that includes this feature.
166166

167167
Consumers must therefore treat a missing bundle as non-fatal (skip + warn), not an error.
168168

169+
## `changelog bundle` entry sourcing (declared-gate)
170+
171+
The `changelog bundle` command aggregates individual changelog **entries**. It can read those
172+
entries from the local folder or fetch a product's published entries from the CDN
173+
(`{product}/changelog/registry.json``{product}/changelog/{file}`, via `CdnChangelogEntryFetcher`).
174+
175+
CDN entry sourcing is **opt-in per product** (a *declared-gate*): a product's entries are pulled from
176+
the CDN only when that product is declared under `release_notes` in the repo's `docset.yml` — the same
177+
declaration the directive consumes. The decision is made per run by `ChangelogBundlingService`:
178+
179+
- **Local folder** when `bundle.use_local_changelogs: true`, when `--directory` is passed, when no
180+
concrete product is in scope (for example `--all` or PR/issue-only filters), or when any in-scope
181+
product is **not** declared under `release_notes`.
182+
- **CDN** only when every in-scope product is declared. The declared set is read with
183+
`DocumentationSetFile.LoadMetadata` from the known docset locations (repo root or `docs/`).
184+
185+
The same gate drives the `--plan` `needs_network` output, so a planning step and the actual bundle
186+
run agree on whether the Docker bundle needs network access. The registry-fetch is fail-fast and an
187+
entry still missing after its retry budget fails the bundle (an incomplete release would otherwise
188+
ship silently). `CdnChangelogEntryFetcher` reuses a shared `HttpClient` in production and disposes an
189+
owned client only when a test injects a handler, mirroring `CdnChangelogFetcher`.
190+
169191
## Consumer: `{changelog}` directive `cdn:` mode (implemented)
170192

171193
### Syntax

src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs

Lines changed: 81 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
33
// See the LICENSE file in the project root for more information
44

5+
using System.Net;
56
using System.Text.Json;
67
using Microsoft.Extensions.Logging;
78

@@ -30,11 +31,7 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes;
3031
/// silently shipping an incomplete release bundle is worse than failing the run.
3132
/// </para>
3233
/// </remarks>
33-
public sealed class CdnChangelogEntryFetcher(
34-
ILoggerFactory logFactory,
35-
HttpMessageHandler? handler = null,
36-
int maxAttempts = CdnChangelogEntryFetcher.DefaultMaxAttempts,
37-
Action<TimeSpan, Cancel>? sleep = null)
34+
public sealed class CdnChangelogEntryFetcher : IDisposable
3835
{
3936
private const int SupportedSchemaVersion = 1;
4037

@@ -43,18 +40,64 @@ public sealed class CdnChangelogEntryFetcher(
4340
private const int BaseRetryDelayMs = 500;
4441
private const int MaxRetryDelayMs = 2000;
4542

46-
private readonly ILogger _logger = logFactory.CreateLogger<CdnChangelogEntryFetcher>();
47-
private readonly HttpClient _httpClient = handler is null ? new HttpClient() : new HttpClient(handler, disposeHandler: false);
48-
private readonly int _maxAttempts = maxAttempts < 1 ? DefaultMaxAttempts : maxAttempts;
49-
private readonly Action<TimeSpan, Cancel> _sleep = sleep ?? DefaultSleep;
43+
/// <summary>
44+
/// Bounds an individual registry/entry HTTP request so a stalled CDN connection cannot hang a bundle run.
45+
/// </summary>
46+
private static readonly TimeSpan FetchTimeout = TimeSpan.FromSeconds(30);
47+
48+
/// <summary>
49+
/// Process-wide client shared by every fetcher built for the production (no injected handler) path.
50+
/// <see cref="HttpClient"/> is thread-safe and intended to be long-lived; a single static instance avoids
51+
/// leaking a socket handle per fetch, and <see cref="SocketsHttpHandler.PooledConnectionLifetime"/>
52+
/// bounds DNS staleness. It is intentionally never disposed — it lives for the lifetime of the process.
53+
/// </summary>
54+
private static readonly HttpClient SharedHttpClient = new(
55+
new SocketsHttpHandler
56+
{
57+
AutomaticDecompression = DecompressionMethods.All,
58+
PooledConnectionLifetime = TimeSpan.FromMinutes(5)
59+
})
60+
{ Timeout = FetchTimeout };
61+
62+
private readonly ILogger _logger;
63+
private readonly HttpClient _httpClient;
64+
private readonly int _maxAttempts;
65+
private readonly Func<TimeSpan, Cancel, Task> _sleep;
66+
67+
/// <summary>
68+
/// Non-null only when a caller injects its own <see cref="HttpMessageHandler"/> (tests): in that case we
69+
/// own a per-instance client and must dispose it. On the production path <see cref="_httpClient"/> points
70+
/// at <see cref="SharedHttpClient"/>, which is never disposed.
71+
/// </summary>
72+
private readonly HttpClient? _ownedHttpClient;
73+
74+
public CdnChangelogEntryFetcher(
75+
ILoggerFactory logFactory,
76+
HttpMessageHandler? handler = null,
77+
int maxAttempts = DefaultMaxAttempts,
78+
Func<TimeSpan, Cancel, Task>? sleep = null)
79+
{
80+
_logger = logFactory.CreateLogger<CdnChangelogEntryFetcher>();
81+
_maxAttempts = maxAttempts < 1 ? DefaultMaxAttempts : maxAttempts;
82+
_sleep = sleep ?? DefaultSleepAsync;
83+
84+
if (handler is null)
85+
_httpClient = SharedHttpClient;
86+
else
87+
{
88+
// disposeHandler: false — the injected handler is owned by the caller (tests), not by us.
89+
_ownedHttpClient = new HttpClient(handler, disposeHandler: false) { Timeout = FetchTimeout };
90+
_httpClient = _ownedHttpClient;
91+
}
92+
}
5093

5194
/// <summary>
5295
/// Downloads the changelog entries for <paramref name="product"/> from the CDN at
5396
/// <paramref name="baseUri"/>. Returns an empty list after emitting an error when the registry cannot
5497
/// be read or when a registry-listed entry cannot be fetched within the retry budget. Entries are
5598
/// returned in registry order; the caller owns filtering and de-duplication.
5699
/// </summary>
57-
public IReadOnlyList<CdnChangelogEntry> Fetch(
100+
public async Task<IReadOnlyList<CdnChangelogEntry>> FetchAsync(
58101
Uri baseUri,
59102
string product,
60103
Action<string> emitError,
@@ -66,7 +109,7 @@ public IReadOnlyList<CdnChangelogEntry> Fetch(
66109
ChangelogRegistry? registry;
67110
try
68111
{
69-
registry = FetchRegistry(registryUri, ctx);
112+
registry = await FetchRegistryAsync(registryUri, ctx).ConfigureAwait(false);
70113
}
71114
catch (Exception ex) when (ex is not OperationCanceledException)
72115
{
@@ -100,7 +143,8 @@ public IReadOnlyList<CdnChangelogEntry> Fetch(
100143
}
101144

102145
var entryUri = Combine(baseUri, product, "changelog", fileName);
103-
if (TryFetchEntry(entryUri, fileName, product, ctx, out var content, out var lastError))
146+
var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, product, ctx).ConfigureAwait(false);
147+
if (fetched)
104148
{
105149
entries.Add(new CdnChangelogEntry(fileName, content));
106150
continue;
@@ -124,20 +168,19 @@ public IReadOnlyList<CdnChangelogEntry> Fetch(
124168
/// up to <see cref="_maxAttempts"/> times with exponential backoff. Retry requests are cache-busted
125169
/// so a CloudFront-cached 404 cannot pin the result for the whole window.
126170
/// </summary>
127-
private bool TryFetchEntry(Uri uri, string fileName, string product, Cancel ctx, out string content, out string? lastError)
171+
private async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string product, Cancel ctx)
128172
{
129-
content = string.Empty;
130-
lastError = null;
173+
string? lastError = null;
131174

132175
for (var attempt = 1; attempt <= _maxAttempts; attempt++)
133176
{
134177
ctx.ThrowIfCancellationRequested();
135178
try
136179
{
137-
content = FetchText(uri, attempt, ctx);
180+
var content = await FetchTextAsync(uri, attempt, ctx).ConfigureAwait(false);
138181
if (attempt > 1)
139182
_logger.LogInformation("Fetched changelog entry '{File}' for {Product} on attempt {Attempt}/{Max}", fileName, product, attempt, _maxAttempts);
140-
return true;
183+
return (true, content, null);
141184
}
142185
catch (Exception ex) when (ex is not OperationCanceledException)
143186
{
@@ -149,36 +192,34 @@ private bool TryFetchEntry(Uri uri, string fileName, string product, Cancel ctx,
149192
_logger.LogDebug(
150193
"Changelog entry '{File}' for {Product} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}",
151194
fileName, product, attempt, _maxAttempts, ex.Message, delay);
152-
_sleep(delay, ctx);
195+
await _sleep(delay, ctx).ConfigureAwait(false);
153196
}
154197
}
155198

156-
return false;
199+
return (false, string.Empty, lastError);
157200
}
158201

159-
private ChangelogRegistry? FetchRegistry(Uri registryUri, Cancel ctx)
202+
private async Task<ChangelogRegistry?> FetchRegistryAsync(Uri registryUri, Cancel ctx)
160203
{
161204
_logger.LogInformation("Fetching changelog entry registry {RegistryUri}", registryUri);
162205
using var request = new HttpRequestMessage(HttpMethod.Get, registryUri);
163-
using var response = _httpClient.Send(request, ctx);
206+
using var response = await _httpClient.SendAsync(request, ctx).ConfigureAwait(false);
164207
_ = response.EnsureSuccessStatusCode();
165-
using var stream = response.Content.ReadAsStream(ctx);
166-
return JsonSerializer.Deserialize(stream, ChangelogRegistryJsonContext.Default.ChangelogRegistry);
208+
await using var stream = await response.Content.ReadAsStreamAsync(ctx).ConfigureAwait(false);
209+
return await JsonSerializer.DeserializeAsync(stream, ChangelogRegistryJsonContext.Default.ChangelogRegistry, ctx).ConfigureAwait(false);
167210
}
168211

169-
private string FetchText(Uri uri, int attempt, Cancel ctx)
212+
private async Task<string> FetchTextAsync(Uri uri, int attempt, Cancel ctx)
170213
{
171214
// Only bust the cache on retries: the first hit should use the CDN cache normally (the common,
172215
// already-propagated case); retries explicitly want to bypass any cached 404.
173216
var requestUri = attempt > 1 ? WithCacheBuster(uri) : uri;
174217
using var request = new HttpRequestMessage(HttpMethod.Get, requestUri);
175218
if (attempt > 1)
176219
_ = request.Headers.TryAddWithoutValidation("Cache-Control", "no-cache");
177-
using var response = _httpClient.Send(request, ctx);
220+
using var response = await _httpClient.SendAsync(request, ctx).ConfigureAwait(false);
178221
_ = response.EnsureSuccessStatusCode();
179-
using var stream = response.Content.ReadAsStream(ctx);
180-
using var reader = new StreamReader(stream);
181-
return reader.ReadToEnd();
222+
return await response.Content.ReadAsStringAsync(ctx).ConfigureAwait(false);
182223
}
183224

184225
private static TimeSpan RetryDelay(int attempt)
@@ -188,10 +229,10 @@ private static TimeSpan RetryDelay(int attempt)
188229
return TimeSpan.FromMilliseconds(ms);
189230
}
190231

191-
private static void DefaultSleep(TimeSpan delay, Cancel ctx)
232+
private static async Task DefaultSleepAsync(TimeSpan delay, Cancel ctx)
192233
{
193234
if (delay > TimeSpan.Zero)
194-
_ = ctx.WaitHandle.WaitOne(delay);
235+
await Task.Delay(delay, ctx).ConfigureAwait(false);
195236
}
196237

197238
private static Uri WithCacheBuster(Uri uri)
@@ -215,4 +256,14 @@ private static bool IsSafeFileName(string fileName) =>
215256
!fileName.Contains('/', StringComparison.Ordinal)
216257
&& !fileName.Contains('\\', StringComparison.Ordinal)
217258
&& fileName is not ("." or "..");
259+
260+
/// <summary>
261+
/// Disposes the per-instance <see cref="HttpClient"/> created for an injected handler. The shared
262+
/// production client (<see cref="SharedHttpClient"/>) is process-lived and intentionally not disposed.
263+
/// </summary>
264+
public void Dispose()
265+
{
266+
_ownedHttpClient?.Dispose();
267+
GC.SuppressFinalize(this);
268+
}
218269
}

0 commit comments

Comments
 (0)