diff --git a/docs/cli/changelog/cmd-note.md b/docs/cli/changelog/cmd-note.md index 2463549d89..7b5efc3838 100644 --- a/docs/cli/changelog/cmd-note.md +++ b/docs/cli/changelog/cmd-note.md @@ -66,7 +66,7 @@ The scrubber writes two indexes per note: `{changelog}` does not read these indexes. It loads published bundle YAML (and amend sidecars listed in `bundle/{product}/registry.json`). -If the release bundle for that product and version or date has already shipped when you upload, the scrubber generates a `{parent}.amend-notes.yaml` sidecar so the changelog reaches published docs without a manual rerun. Don't run `changelog bundle-amend --add` for that file. Refer to [](/data/release-notes/bundle.md#changelog-bundle-notes-after-ship). +If the release bundle for that product and version or date has already shipped when you upload, the scrubber generates an amend sidecar for **that product**, then rebuilds that product's `bundle/{product}/registry.json` so `{changelog}` `:cdn:` pages pick it up. Other products that share the version are left alone. Don't run `changelog bundle-amend --add` for that file. Refer to [](/data/release-notes/bundle.md#changelog-bundle-notes-after-ship). An empty product-scoped notes index stays until that registry rebuild succeeds, so a failed registry write can retry with the vanished product still in the notes map. If there is no existing or planned bundle for that product and version or date, you can create a bundle from a path list that contains all the relevant changelogs. Refer to [Bundle by file paths](/cli/changelog/bundle.md#changelog-bundle-files). diff --git a/docs/development/changelog-bundle-registry.md b/docs/development/changelog-bundle-registry.md index 3affe8e2a7..daee6651b0 100644 --- a/docs/development/changelog-bundle-registry.md +++ b/docs/development/changelog-bundle-registry.md @@ -71,11 +71,15 @@ narrowed reconciliation to the bundle tree): - **Amend-notes sidecars** — `bundle/{product}/{parent}.amend-notes.yaml`, also **public bucket only**, authored by the scrubber Lambda's `NoteAmendReconciler`. When a note is uploaded after its release bundle has already shipped, the reconciler generates one aggregate sidecar per - published bundle that lists all such late notes. The Lambda rebuilds it from current state on - every reconcile, so redelivered events never produce duplicate amends. `{changelog}` `:cdn:` and - `changelog render` merge this sidecar into the parent the same way as numbered `.amend-{N}` - files, after those numbered amends. The `.amend-notes` suffix - is **reserved** — do not create files with that suffix manually; see + published bundle that lists all such late notes **for that product**. The Lambda rebuilds it + from current state on every reconcile, so redelivered events never produce duplicate amends. + After a write, skip-unchanged, or delete of that sidecar, the same pass rebuilds + `bundle/{product}/registry.json` and the bundle shallow map so `{changelog}` `:cdn:` can + discover it. An empty product-scoped notes index is removed only after that registry + rebuild succeeds. Other products at the same version are not walked and their sidecars are not + deleted. `{changelog}` `:cdn:` and `changelog render` merge this sidecar into the parent the + same way as numbered `.amend-{N}` files, after those numbered amends. The `.amend-notes` + suffix is **reserved** — do not create files with that suffix manually; see [](/cli/changelog/bundle-amend.md). Public copies track private-bucket create and delete events. Authors cannot issue those deletes through docs-builder today: `changelog upload` does not delete objects, and `changelog remove` is local-only. For the author-facing @@ -169,8 +173,9 @@ amend sidecars. Do not hand-edit `notes-{version}.json` or `.amend-notes` sideca delete pool objects through docs-builder today. A 404 on both the product-scoped index and the legacy version-union index means "no notes -published for this product and version". An empty `notes` array never appears on a successfully -reconciled index — the index is deleted rather than emptied, following the same +published for this product and version". An empty `notes` array is not the durable form of a +successfully reconciled index: after sidecar work and a successful product registry rebuild, +the empty product-scoped index is deleted rather than rewritten empty, following the same [absent ≠ empty](#absent-empty) rule as the bundle registry. Until older clients stop reading `notes-{version}.json`, the Lambda keeps that key while any note still declares the version. diff --git a/src/services/Elastic.Changelog/Reconciliation/NoteAmendReconciler.cs b/src/services/Elastic.Changelog/Reconciliation/NoteAmendReconciler.cs index 313387d953..cc65f9f154 100644 --- a/src/services/Elastic.Changelog/Reconciliation/NoteAmendReconciler.cs +++ b/src/services/Elastic.Changelog/Reconciliation/NoteAmendReconciler.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.Concurrent; using System.Net; using System.Text.Json; using Amazon.S3; @@ -14,11 +15,12 @@ namespace Elastic.Changelog.Reconciliation; /// -/// Compares each note in the per-version notes indexes against published bundles for the same -/// version, and either creates or deletes a reconciler-owned amend sidecar +/// Compares each note in the product-scoped notes indexes against that product's published +/// bundles for the same version, and either creates or deletes a reconciler-owned amend sidecar /// ({parent}.amend-notes.yaml) that carries notes that arrived after the release shipped. -/// Also updates each bundle_seq in the notes index: 0 = no bundle yet, 1 = shipped in the -/// original bundle or a human amend, 2 = carried by the reconciler amend sidecar. +/// Also updates each bundle_seq on the product-scoped notes index: 0 = no bundle yet, +/// 1 = shipped in the original bundle or a human amend, 2 = carried by the reconciler amend sidecar. +/// The dual-written legacy notes-{version}.json path list is refreshed in the same pass. /// /// /// @@ -49,87 +51,152 @@ public sealed class NoteAmendReconciler( private readonly TimeSpan _retryBaseDelay = retryBaseDelay ?? TimeSpan.FromMilliseconds(200); /// - /// For the given repository scope, scans every product's bundle registry to determine which - /// notes have shipped and which are late, writes or deletes the reconciler-owned amend sidecars, - /// and re-writes the notes indexes with correct bundle_seq values. + /// For the given repository scope, scans each product that appears in + /// (not every bundle/{product}/ prefix), writes or + /// deletes that product's reconciler-owned amend sidecars, and re-writes product-scoped and + /// legacy notes indexes with correct bundle_seq values. Empty version lists mean that + /// product×version just vanished from the notes index; amend still runs so sidecars can drop. /// - /// The notes scope for this repo. - /// Output of . - /// Cancellation token. - public async Task ReconcileAsync( + /// + /// Products that had an amend-notes write, skip-unchanged, or delete (including when the + /// sidecar was already absent), plus empty product-scoped notes-index keys to delete after + /// those products' registry rebuild succeeds. Products that were not in the notes map are + /// not returned and are not swept. + /// + public async Task ReconcileAsync( ChangelogScope notesScope, - IReadOnlyDictionary> notesByVersion, + IReadOnlyDictionary>> notesByProduct, Cancel ctx ) { - if (notesByVersion.Count == 0) - return; + if (notesByProduct.Count == 0) + return new NoteAmendOutcome([], []); var groupParts = notesScope.Group.Split('/'); var (org, repo) = (groupParts[0], groupParts[1]); - // Track bundle_seq for each (version → path → seq). Default 0 = unreleased. var seqMap = new Dictionary>(StringComparer.Ordinal); - foreach (var (version, notes) in notesByVersion) - seqMap[version] = notes.ToDictionary(n => n.Path, _ => 0, StringComparer.Ordinal); + foreach (var (product, byVersion) in notesByProduct) + { + foreach (var (version, notes) in byVersion) + seqMap[ProductVersionKey(product, version)] = notes.ToDictionary(n => n.Path, _ => 0, StringComparer.Ordinal); + } - // List all product names from the bundle tree. - var products = await ListBundleProductsAsync(ctx); - _logger.LogDebug("NoteAmendReconciler: scanning {Count} bundle product(s) for repo {Org}/{Repo}", products.Count, org, repo); + _logger.LogDebug( + "NoteAmendReconciler: scanning {Count} product(s) from the notes index for repo {Org}/{Repo}", + notesByProduct.Count, + org, + repo + ); - foreach (var product in products) + var touched = new HashSet(StringComparer.Ordinal); + foreach (var (product, byVersion) in notesByProduct) { ctx.ThrowIfCancellationRequested(); - await ProcessProductAsync(org, repo, product, notesByVersion, seqMap, ctx); + if (await ProcessProductAsync(org, repo, product, byVersion, seqMap, ctx)) + _ = touched.Add(product); } - // Re-write notes indexes with the updated bundle_seq values. - await Parallel.ForEachAsync(notesByVersion, new ParallelOptions + var emptyIndexes = await RewriteNotesIndexesAsync(org, repo, notesByProduct, seqMap, touched, ctx); + return new NoteAmendOutcome([.. touched], emptyIndexes); + } + + private async Task> RewriteNotesIndexesAsync( + string org, + string repo, + IReadOnlyDictionary>> notesByProduct, + IReadOnlyDictionary> seqMap, + HashSet touchedProducts, + Cancel ctx + ) + { + var productWrites = notesByProduct.SelectMany(p => p.Value.Select(v => (Product: p.Key, Version: v.Key, Notes: v.Value))).ToList(); + var emptyIndexes = new ConcurrentBag(); + + await Parallel.ForEachAsync(productWrites, new ParallelOptions { MaxDegreeOfParallelism = MaxParallelWrites, CancellationToken = ctx - }, async (kvp, ct) => + }, async (write, ct) => { - var (version, notes) = kvp; - var seqs = seqMap[version]; - var updatedEntries = notes - .Select(n => n with { BundleSeq = seqs.TryGetValue(n.Path, out var s) ? s : 0 }) - .OrderBy(n => n.Path, StringComparer.Ordinal) - .ToList(); - var indexKey = ChangelogKeys.NotesIndexKey(org, repo, version); - await notesIndexReconciler.WriteIndexAsync(indexKey, updatedEntries, ct); + var indexKey = ChangelogKeys.NotesIndexKey(org, repo, write.Product, write.Version); + if (write.Notes.Count == 0) + { + if (touchedProducts.Contains(write.Product)) + emptyIndexes.Add(new NoteAmendEmptyIndex(write.Product, indexKey)); + return; + } + + var seqs = seqMap[ProductVersionKey(write.Product, write.Version)]; + var updatedEntries = WithSeqs(write.Notes, seqs); + await notesIndexReconciler.WriteIndexAsync(indexKey, updatedEntries, ct, new NotesIndexMetadata(write.Product, write.Version)); }); + + foreach (var (version, unionNotes) in UnionByVersion(notesByProduct)) + { + ctx.ThrowIfCancellationRequested(); + if (unionNotes.Count == 0) + continue; + + var seqs = MaxSeqsForVersion(notesByProduct.Keys, version, seqMap); + var updatedEntries = WithSeqs(unionNotes, seqs); + var indexKey = ChangelogKeys.NotesIndexKey(org, repo, version); + await notesIndexReconciler.WriteIndexAsync(indexKey, updatedEntries, ctx); + } + + return [.. emptyIndexes]; } - // ----------------------------------------------------------------------------------------- - // Product scanning - // ----------------------------------------------------------------------------------------- + private static List WithSeqs(IReadOnlyList notes, IReadOnlyDictionary seqs) => + notes + .Select(n => n with { BundleSeq = seqs.TryGetValue(n.Path, out var s) ? s : 0 }) + .OrderBy(n => n.Path, StringComparer.Ordinal) + .ToList(); - private async Task> ListBundleProductsAsync(Cancel ctx) + private static Dictionary> UnionByVersion( + IReadOnlyDictionary>> notesByProduct + ) { - var products = new List(); - var request = new ListObjectsV2Request { BucketName = publicBucketName, Prefix = ChangelogKeys.BundlePrefix, Delimiter = "/" }; - - ListObjectsV2Response response; - do + var byVersion = new Dictionary>(StringComparer.Ordinal); + foreach (var byProductVersion in notesByProduct.Values) { - response = await s3Client.ListObjectsV2Async(request, ctx); - foreach (var prefix in response.CommonPrefixes ?? []) + foreach (var (version, notes) in byProductVersion) { - // CommonPrefix is like "bundle/elasticsearch/" — strip the outer segments. - var inner = prefix[ChangelogKeys.BundlePrefix.Length..]; - var product = inner.TrimEnd('/'); - if (!string.IsNullOrEmpty(product)) - products.Add(product); + if (!byVersion.TryGetValue(version, out var union)) + byVersion[version] = union = []; + foreach (var note in notes) + { + if (!union.Any(e => e.Path == note.Path)) + union.Add(note); + } } - request.ContinuationToken = response.NextContinuationToken; } - while (response.IsTruncated == true); + return byVersion; + } - return products; + private static Dictionary MaxSeqsForVersion( + IEnumerable products, + string version, + IReadOnlyDictionary> seqMap + ) + { + var max = new Dictionary(StringComparer.Ordinal); + foreach (var product in products) + { + if (!seqMap.TryGetValue(ProductVersionKey(product, version), out var seqs)) + continue; + foreach (var (path, seq) in seqs) + { + if (!max.TryGetValue(path, out var current) || seq > current) + max[path] = seq; + } + } + return max; } - private async Task ProcessProductAsync( + private static string ProductVersionKey(string product, string version) => $"{product}/{version}"; + + private async Task ProcessProductAsync( string org, string repo, string product, @@ -152,23 +219,22 @@ Cancel ctx } catch (AmazonS3Exception ex) when (ex.StatusCode == HttpStatusCode.NotFound) { - return; + return false; } catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogWarning(ex, "Could not read bundle registry for product {Product}; skipping", product); - return; + return false; } if (registry is null || registry.Bundles.Count == 0) - return; + return false; - // For each version that has notes, look for a matching parent bundle. + var touched = false; foreach (var (version, notes) in notesByVersion) { ctx.ThrowIfCancellationRequested(); - // Parent bundle: not an amend file, target matches the version. var parentBundle = registry.Bundles.FirstOrDefault( b => !string.IsNullOrEmpty(b.File) && !BundleAmendMerger.IsAmendFile(b.File) && ChangelogVersionMatch.Matches( version, @@ -178,13 +244,27 @@ Cancel ctx ); if (parentBundle is null) - continue; // No bundle yet → every note stays at bundle_seq 0. - - await ProcessVersionBundleAsync(org, repo, product, parentBundle, registry, version, notes, seqMap[version], ctx); + continue; + + if ( + await ProcessVersionBundleAsync( + org, + repo, + product, + parentBundle, + registry, + version, + notes, + seqMap[ProductVersionKey(product, version)], + ctx + ) + ) + touched = true; } + return touched; } - private async Task ProcessVersionBundleAsync( + private async Task ProcessVersionBundleAsync( string org, string repo, string product, @@ -201,7 +281,7 @@ Cancel ctx var parent = await TryReadBundleAsync(parentKey, ctx); if (parent is null) - return; + return false; // A bundle with no file-annotated entries (hand-authored / legacy) has no reliable // shipped set — skip to avoid false positives. @@ -213,7 +293,7 @@ Cancel ctx parentKey, version ); - return; + return false; } // Read existing numeric amend bundles (in order) to compute the full merged set. @@ -271,17 +351,16 @@ Cancel ctx { var amendBundle = AmendDocumentBuilder.Build(parent.Products, lateEntries, []); var newJson = ReleaseNotesSerialization.SerializeBundle(amendBundle); - await WriteAmendNotesAsync(amendNotesKey, newJson, ctx); + _ = await WriteAmendNotesAsync(amendNotesKey, newJson, ctx); foreach (var note in lateNotes) - seqByPath[note.Path] = 2; // carried by the reconciler amend + seqByPath[note.Path] = 2; + return true; } + return false; } - else - { - // All notes are shipped — delete the sidecar if it exists. - await DeleteAmendNotesIfExistsAsync(amendNotesKey, ctx); - } + + return await DeleteAmendNotesIfExistsAsync(amendNotesKey, ctx); } // ----------------------------------------------------------------------------------------- @@ -331,7 +410,7 @@ Cancel ctx // Conditional S3 write / delete // ----------------------------------------------------------------------------------------- - private async Task WriteAmendNotesAsync(string key, string newJson, Cancel ctx) + private async Task WriteAmendNotesAsync(string key, string newJson, Cancel ctx) { const int maxAttempts = 5; for (var attempt = 1; attempt <= maxAttempts; attempt++) @@ -366,7 +445,7 @@ private async Task WriteAmendNotesAsync(string key, string newJson, Cancel ctx) if (existingJson == newJson) { _logger.LogDebug("Amend-notes {Key} is unchanged; skipping write", key); - return; + return true; } } catch (AmazonS3Exception ex) when (ex.StatusCode == HttpStatusCode.NotFound) @@ -390,7 +469,7 @@ private async Task WriteAmendNotesAsync(string key, string newJson, Cancel ctx) _ = await s3Client.PutObjectAsync(putRequest, ctx); _metrics.IncrementRegistryWrites(); _logger.LogInformation("Wrote amend-notes sidecar {Key}", key); - return; + return true; } catch (AmazonS3Exception ex) when (ex.StatusCode is HttpStatusCode.PreconditionFailed || (int)ex.StatusCode == 409) { @@ -411,9 +490,10 @@ private async Task WriteAmendNotesAsync(string key, string newJson, Cancel ctx) _logger.LogDebug(ex, "Amend-notes write {Key} failed (attempt {A}/{Max}); retrying", key, attempt, maxAttempts); } } + return false; } - private async Task DeleteAmendNotesIfExistsAsync(string key, Cancel ctx) + private async Task DeleteAmendNotesIfExistsAsync(string key, Cancel ctx) { try { @@ -425,19 +505,17 @@ private async Task DeleteAmendNotesIfExistsAsync(string key, Cancel ctx) _ = await s3Client.DeleteObjectAsync(new DeleteObjectRequest { BucketName = publicBucketName, Key = key, IfMatch = etag }, ctx); _logger.LogInformation("Deleted stale amend-notes sidecar {Key}", key); + return true; } catch (AmazonS3Exception ex) when (ex.StatusCode == HttpStatusCode.NotFound) { - // Nothing to delete — this is the expected steady state when all notes are shipped. + _logger.LogDebug("Amend-notes {Key} already absent; treating delete as done so registry rebuild can retry", key); + return true; } catch (AmazonS3Exception ex) when (ex.StatusCode == HttpStatusCode.PreconditionFailed) { - // Another reconciler deleted or replaced it concurrently — safe to ignore. _logger.LogDebug("Amend-notes {Key} was updated concurrently; delete skipped", key); - } - catch (Exception ex) when (ex is not OperationCanceledException) - { - _logger.LogWarning(ex, "Could not delete stale amend-notes sidecar {Key}; will retry on next reconcile", key); + return true; } } @@ -480,3 +558,12 @@ private async Task DeleteAmendNotesIfExistsAsync(string key, Cancel ctx) return slash >= 0 ? normalized[(slash + 1)..] : normalized; } } + +/// +/// Products whose amend sidecar was written, skipped as unchanged, or deleted, plus product-scoped +/// notes-index keys that are empty and must be deleted after those products' registry rebuild succeeds. +/// +public sealed record NoteAmendOutcome(IReadOnlyList TouchedProducts, IReadOnlyList EmptyProductIndexes); + +/// A product-scoped notes index with no remaining notes. +public sealed record NoteAmendEmptyIndex(string Product, string Key); diff --git a/src/services/Elastic.Changelog/Reconciliation/NotesIndexReconciler.cs b/src/services/Elastic.Changelog/Reconciliation/NotesIndexReconciler.cs index 1e8f2d1f52..d3c4707865 100644 --- a/src/services/Elastic.Changelog/Reconciliation/NotesIndexReconciler.cs +++ b/src/services/Elastic.Changelog/Reconciliation/NotesIndexReconciler.cs @@ -48,10 +48,16 @@ public sealed class NotesIndexReconciler( /// read to derive the grouping; every affected index is then (re)written. /// /// - /// A map of version → list of NoteIndexEntry (the version-union, same shape NoteAmend - /// consumes today). Returns an empty dictionary when no notes exist. + /// Notes grouped by product, then version. Legacy version-union indexes are still written + /// in this pass but are not part of the return value; uses + /// the product map only. A product×version whose product-scoped index is stale is included + /// with an empty note list so amend can drop that product's sidecar. Those keys stay in S3 + /// until amend succeeds; leftover version-union indexes are deleted in this pass. /// - public async Task>> ReconcileRepoAsync(ChangelogScope notesScope, Cancel ctx) + public async Task>>> ReconcileRepoAsync( + ChangelogScope notesScope, + Cancel ctx + ) { if (notesScope.Kind != ChangelogScopeKind.Notes) throw new ArgumentException($"Notes reconcile requires a Notes scope; got '{notesScope}'.", nameof(notesScope)); @@ -82,43 +88,67 @@ public async Task>> Re var existingIndexKeys = await ListExistingNotesIndexes(notesScope, ctx); var intendedKeys = IntendedIndexKeys(org, repo, byProductVersion.Keys, byVersion.Keys); - if (byVersion.Count == 0) - { - _logger.LogDebug("No versions found for repo {Repo}; removing any stale indexes", notesScope.Group); - await DeleteStaleIndexes(existingIndexKeys, intendedKeys, ctx); - return new Dictionary>(); - } - - var writes = BuildIndexWrites(org, repo, byProductVersion, byVersion); - var written = new Dictionary>(StringComparer.Ordinal); + var writtenProducts = new Dictionary>>(StringComparer.Ordinal); + IReadOnlyList<(string Product, string Version)> vanished = []; try { - await Parallel.ForEachAsync(writes, new ParallelOptions + if (byVersion.Count == 0) { - MaxDegreeOfParallelism = MaxParallelReads, - CancellationToken = ctx - }, async (write, ct) => + _logger.LogDebug("No versions found for repo {Repo}; removing any stale indexes", notesScope.Group); + } + else { - await WriteIndexAsync( - write.Key, - write.Entries, - ct, - write.Product is null ? null : new NotesIndexMetadata(write.Product, write.Version!) - ); - if (write.Product is not null) - return; - lock (written) - written[write.Version!] = write.Entries; - }); + var writes = BuildIndexWrites(org, repo, byProductVersion, byVersion); + await Parallel.ForEachAsync(writes, new ParallelOptions + { + MaxDegreeOfParallelism = MaxParallelReads, + CancellationToken = ctx + }, async (write, ct) => + { + await WriteIndexAsync( + write.Key, + write.Entries, + ct, + write.Product is null ? null : new NotesIndexMetadata(write.Product, write.Version!) + ); + if (write.Product is null) + return; + lock (writtenProducts) + { + if (!writtenProducts.TryGetValue(write.Product, out var byVer)) + writtenProducts[write.Product] = byVer = [with(StringComparer.Ordinal)]; + byVer[write.Version!] = write.Entries; + } + }); + } } finally { - await DeleteStaleIndexes(existingIndexKeys, intendedKeys, ctx); + vanished = await DeleteStaleIndexes(existingIndexKeys, intendedKeys, ctx); } - return written; + MergeVanishedProductVersions(writtenProducts, vanished); + return ToProductMap(writtenProducts); } + private static void MergeVanishedProductVersions( + Dictionary>> map, + IReadOnlyList<(string Product, string Version)> vanished + ) + { + foreach (var (product, version) in vanished) + { + if (!map.TryGetValue(product, out var byVer)) + map[product] = byVer = [with(StringComparer.Ordinal)]; + if (!byVer.ContainsKey(version)) + byVer[version] = []; + } + } + + private static IReadOnlyDictionary>> ToProductMap( + Dictionary>> map + ) => map.ToDictionary(kv => kv.Key, kv => (IReadOnlyDictionary>)kv.Value, StringComparer.Ordinal); + private static void AddIndexEntry(Dictionary> map, string groupKey, string poolRelativePath) { if (!map.TryGetValue(groupKey, out var entries)) @@ -196,13 +226,28 @@ private async Task> ListExistingNotesIndexes(ChangelogScop return keys; } - private async Task DeleteStaleIndexes(IReadOnlyList existingKeys, HashSet intendedKeys, Cancel ctx) + private async Task> DeleteStaleIndexes( + IReadOnlyList existingKeys, + HashSet intendedKeys, + Cancel ctx + ) { + var vanished = new List<(string Product, string Version)>(); foreach (var key in existingKeys) { if (intendedKeys.Contains(key)) continue; + var read = await ReadStaleIndexIdentity(key, ctx); + if (read.Kind is StaleIndexReadKind.ReadFailed or StaleIndexReadKind.AlreadyGone) + continue; + + if (read.Kind == StaleIndexReadKind.ProductScoped) + { + vanished.Add((read.Product!, read.Version!)); + continue; + } + try { _ = await s3Client.DeleteObjectAsync(new DeleteObjectRequest { BucketName = publicBucketName, Key = key }, ctx); @@ -213,6 +258,37 @@ private async Task DeleteStaleIndexes(IReadOnlyList existingKeys, HashSe _logger.LogWarning(ex, "Failed to delete stale notes index {Key}", key); } } + + return vanished; + } + + /// + /// Reads identity from a stale notes-index body. Does not parse the filename slug. + /// A failed GET/deserialize must not delete the object; a legacy body (no product/version) + /// may be deleted without a vanished product. + /// + private async Task ReadStaleIndexIdentity(string key, Cancel ctx) + { + try + { + using var response = await s3Client.GetObjectAsync(new GetObjectRequest { BucketName = publicBucketName, Key = key }, ctx); + await using var stream = response.ResponseStream; + var index = await JsonSerializer.DeserializeAsync(stream, NotesIndexJsonContext.Default.NotesIndex, ctx); + if (index?.Product is not { Length: > 0 } product || index.Version is not { Length: > 0 } version) + return new StaleIndexRead(StaleIndexReadKind.Legacy); + if (!ChangelogKeys.IsValidProduct(product) || !ChangelogKeys.IsValidRepo(version)) + return new StaleIndexRead(StaleIndexReadKind.Legacy); + return new StaleIndexRead(StaleIndexReadKind.ProductScoped, product, version); + } + catch (AmazonS3Exception ex) when (ex.StatusCode == HttpStatusCode.NotFound) + { + return new StaleIndexRead(StaleIndexReadKind.AlreadyGone); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Could not read identity from stale notes index {Key}; leaving it for the next reconcile", key); + return new StaleIndexRead(StaleIndexReadKind.ReadFailed); + } } private async Task> ListNoteFiles(ChangelogScope notesScope, Cancel ctx) @@ -330,6 +406,23 @@ private static bool IsNoteFileName(string fileName) => } } + /// + /// Deletes a notes-index object. A missing key is success so a retry can drop a vanished + /// product's index after registry rebuild without racing a prior delete. + /// + public async Task DeleteIndexAsync(string key, Cancel ctx) + { + try + { + _ = await s3Client.DeleteObjectAsync(new DeleteObjectRequest { BucketName = publicBucketName, Key = key }, ctx); + _logger.LogInformation("Removed notes index {Key}", key); + } + catch (AmazonS3Exception ex) when (ex.StatusCode == HttpStatusCode.NotFound) + { + _logger.LogDebug("Notes index {Key} already absent", key); + } + } + /// /// Writes the notes index with conditional S3 writes (If-Match / If-None-Match) to guard against /// concurrent reconcile races, mirroring the pattern used by . @@ -454,6 +547,16 @@ public async Task WriteIndexAsync(string key, IReadOnlyList entr } private readonly record struct NotesIndexWrite(string Key, IReadOnlyList Entries, string? Product, string? Version); + + private enum StaleIndexReadKind + { + ReadFailed, + AlreadyGone, + Legacy, + ProductScoped + } + + private readonly record struct StaleIndexRead(StaleIndexReadKind Kind, string? Product = null, string? Version = null); } /// Optional product and version written onto a product-scoped notes index body. diff --git a/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs b/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs index fb6f5bd138..b79e89ef02 100644 --- a/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs +++ b/src/services/Elastic.Changelog/Scrubbing/ScrubberProcessor.cs @@ -140,8 +140,22 @@ public async Task> ProcessAsync(IReadOnlyList shallowWork return; } - // Notes indexes (notes-{target}.json) are reconciler-owned; a client that uploads one is + // Notes indexes are reconciler-owned; a client that uploads one is // rejected here — the reconciler writes directly to the public bucket, so no copy is needed. if (ChangelogKeys.IsNotesIndex(key)) { diff --git a/tests/Elastic.Changelog.Tests/Reconciliation/FakeS3.cs b/tests/Elastic.Changelog.Tests/Reconciliation/FakeS3.cs index eb473b305b..21b6b1bd52 100644 --- a/tests/Elastic.Changelog.Tests/Reconciliation/FakeS3.cs +++ b/tests/Elastic.Changelog.Tests/Reconciliation/FakeS3.cs @@ -49,6 +49,12 @@ internal sealed class FakeS3 /// Runs before each DeleteObject is evaluated, with the 1-based call number. public Action? BeforeDelete { get; set; } + /// When set, thrown from DeleteObject for that key before the store is mutated. + public Func? DeleteFault { get; set; } + + /// When set, thrown from PutObject for that key before the store is mutated. + public Func? PutFault { get; set; } + /// Runs after a GetObject resolved its content (which is returned unchanged), with the key and 1-based call number — simulates the source changing right after a read. public Action? AfterGet { get; set; } @@ -204,6 +210,9 @@ private PutObjectResponse Put(PutObjectRequest request) lock (_lock) n = ++_puts; BeforePut?.Invoke(n); + var fault = PutFault?.Invoke(request.Key); + if (fault is not null) + throw fault; lock (_lock) { @@ -227,6 +236,9 @@ private DeleteObjectResponse Delete(DeleteObjectRequest request) lock (_lock) n = ++_deletes; BeforeDelete?.Invoke(n); + var fault = DeleteFault?.Invoke(request.Key); + if (fault is not null) + throw fault; lock (_lock) { diff --git a/tests/Elastic.Changelog.Tests/Reconciliation/NoteAmendReconcilerTests.cs b/tests/Elastic.Changelog.Tests/Reconciliation/NoteAmendReconcilerTests.cs index d2c6c29a91..7f7b31e614 100644 --- a/tests/Elastic.Changelog.Tests/Reconciliation/NoteAmendReconcilerTests.cs +++ b/tests/Elastic.Changelog.Tests/Reconciliation/NoteAmendReconcilerTests.cs @@ -2,7 +2,9 @@ // 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 Amazon.S3; using AwesomeAssertions; using Elastic.Changelog.Reconciliation; using Elastic.Documentation; @@ -104,12 +106,21 @@ private static string RegistryJsonWithTarget(string target, params string[] file private static NotesIndex ReadNotesIndex(string json) => JsonSerializer.Deserialize(json, NotesIndexJsonContext.Default.NotesIndex)!; - private static IReadOnlyDictionary> NotesByVersion(string version, params string[] paths) => - new Dictionary> + private static IReadOnlyDictionary>> NotesByProduct( + string product, + string version, + params string[] paths + ) => + new Dictionary>>(StringComparer.Ordinal) { - [version] = [.. paths.Select(p => new NoteIndexEntry { Path = p, BundleSeq = 0 })] + [product] = new Dictionary>(StringComparer.Ordinal) + { + [version] = [.. paths.Select(p => new NoteIndexEntry { Path = p, BundleSeq = 0 })] + } }; + private static string ProductIndexKey(string? product = null) => ChangelogKeys.NotesIndexKey(Org, Repo, product ?? Product, Version); + // ----------------------------------------------------------------------------------------- [Fact] @@ -120,8 +131,8 @@ public async Task LateNote_NoBundleAmendYet_WritesAmendNotesSidecar() _s3.Seed(PublicBucket, BundleKey(parent), ParentBundleYaml("main/pr-100.yaml")); _s3.Seed(PublicBucket, NoteKey("main", "note-cve.yml"), NoteYaml); - var notesByVersion = NotesByVersion(Version, "main/note-cve.yml"); - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + var notesByProduct = NotesByProduct(Product, Version, "main/note-cve.yml"); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); // Amend sidecar must have been written. _s3.Exists(PublicBucket, AmendNotesKey(parent)).Should().BeTrue("late note must produce an amend sidecar"); @@ -131,6 +142,11 @@ public async Task LateNote_NoBundleAmendYet_WritesAmendNotesSidecar() _s3.Exists(PublicBucket, indexKey).Should().BeTrue("notes index must be re-written with bundle_seq values"); var index = ReadNotesIndex(_s3.ContentOf(PublicBucket, indexKey)); index.Notes.Should().ContainSingle().Which.BundleSeq.Should().Be(2); + + var productIndex = ReadNotesIndex(_s3.ContentOf(PublicBucket, ProductIndexKey())); + productIndex.Notes.Should().ContainSingle().Which.BundleSeq.Should().Be(2); + productIndex.Product.Should().Be(Product); + productIndex.Version.Should().Be(Version); } [Fact] @@ -142,8 +158,8 @@ public async Task NoteShippedInParent_NoAmendWritten_SeqIsOne() _s3.Seed(PublicBucket, BundleKey(parent), ParentBundleYaml("main/note-cve.yml", "main/pr-100.yaml")); _s3.Seed(PublicBucket, NoteKey("main", "note-cve.yml"), NoteYaml); - var notesByVersion = NotesByVersion(Version, "main/note-cve.yml"); - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + var notesByProduct = NotesByProduct(Product, Version, "main/note-cve.yml"); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); // No amend sidecar should be written. _s3.Exists(PublicBucket, AmendNotesKey(parent)).Should().BeFalse("note already in parent → no amend needed"); @@ -180,8 +196,8 @@ public async Task NoteShippedInHumanAmend_NoAmendNotesWritten_SeqIsOne() _s3.Seed(PublicBucket, BundleKey(humanAmend), ReleaseNotesSerialization.SerializeBundle(amendBundle)); _s3.Seed(PublicBucket, NoteKey("main", "note-cve.yml"), NoteYaml); - var notesByVersion = NotesByVersion(Version, "main/note-cve.yml"); - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + var notesByProduct = NotesByProduct(Product, Version, "main/note-cve.yml"); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); _s3 .Exists(PublicBucket, AmendNotesKey(parent)) @@ -208,8 +224,8 @@ public async Task ParentBundleHasNoFileAnnotations_Skipped_SeqRemainsZero() _s3.Seed(PublicBucket, BundleKey(parent), ReleaseNotesSerialization.SerializeBundle(handAuthored)); _s3.Seed(PublicBucket, NoteKey("main", "note-cve.yml"), NoteYaml); - var notesByVersion = NotesByVersion(Version, "main/note-cve.yml"); - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + var notesByProduct = NotesByProduct(Product, Version, "main/note-cve.yml"); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); // No amend written; shipped state is unknown. _s3.Exists(PublicBucket, AmendNotesKey(parent)).Should().BeFalse("unknown shipped state → skip, no amend"); @@ -244,11 +260,129 @@ public async Task NoteRemovedFromIndex_ExistingAmendSidecarDeleted() _s3.Seed(PublicBucket, AmendNotesKey(parent), ReleaseNotesSerialization.SerializeBundle(staleAmend)); // No notes for this version (note was deleted from the pool). - var notesByVersion = NotesByVersion(Version); - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + var notesByProduct = NotesByProduct(Product, Version); + var outcome = await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); _s3.Exists(PublicBucket, AmendNotesKey(parent)).Should().BeFalse("stale amend sidecar must be deleted when no notes remain"); - _s3.Deletes.Should().ContainSingle().Which.Key.Should().Be(AmendNotesKey(parent)); + _s3.Deletes.Select(d => d.Key).Should().Contain(AmendNotesKey(parent)); + _s3.Deletes.Select(d => d.Key).Should().NotContain(ProductIndexKey()); + outcome.TouchedProducts.Should().Equal(Product); + outcome.EmptyProductIndexes.Should().Equal(new NoteAmendEmptyIndex(Product, ProductIndexKey())); + } + + [Fact] + public async Task SidecarAlreadyAbsent_NoLateNotes_ProductStillTouched() + { + const string parent = "elasticsearch-9.3.0.yaml"; + _s3.Seed(PublicBucket, RegistryKey(), RegistryJson(parent)); + _s3.Seed(PublicBucket, BundleKey(parent), ParentBundleYaml("main/pr-100.yaml")); + + var notesByProduct = NotesByProduct(Product, Version); + var outcome = await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); + + _s3.Exists(PublicBucket, AmendNotesKey(parent)).Should().BeFalse(); + outcome.TouchedProducts.Should().Equal(Product); + outcome.EmptyProductIndexes.Should().Equal(new NoteAmendEmptyIndex(Product, ProductIndexKey())); + } + + [Fact] + public async Task ReconcileRepoOmitsProduct_ExistingAmendSidecarDeleted() + { + const string parent = "elasticsearch-9.3.0.yaml"; + _s3.Seed(PublicBucket, RegistryKey(), RegistryJson(parent)); + _s3.Seed(PublicBucket, BundleKey(parent), ParentBundleYaml("main/pr-100.yaml")); + var staleAmend = new Bundle + { + Products = [new BundledProduct(Product, target: Version, lifecycle: Lifecycle.Ga)], + Entries = + [ + new BundledEntry + { + File = new BundledFile { Name = "main/note-cve.yml", Checksum = "old" }, + Title = "CVE", + Type = ChangelogEntryType.Security + } + ] + }; + _s3.Seed(PublicBucket, AmendNotesKey(parent), ReleaseNotesSerialization.SerializeBundle(staleAmend)); + _s3.Seed( + PublicBucket, + ChangelogKeys.NotesIndexKey(Org, Repo, Product, Version), + /*lang=json,strict*/ + """{"schema_version":1,"product":"elasticsearch","version":"9.3.0","notes":[{"path":"main/note-cve.yml","bundle_seq":2}]}""" + ); + _s3.Seed( + PublicBucket, + $"bundle/kibana/registry.json", + JsonSerializer.Serialize( + new ChangelogRegistry + { + Product = "kibana", + Bundles = [new ChangelogRegistryBundle { File = "kibana-9.3.0.yaml", Target = Version }] + }, + ChangelogRegistryJsonContext.Default.ChangelogRegistry + ) + ); + _s3.Seed(PublicBucket, "bundle/kibana/kibana-9.3.0.yaml", ParentBundleFor("kibana", Version, "main/note-kibana.yml")); + _s3.Seed( + PublicBucket, + NoteKey("main", "note-kibana.yml"), + "title: Kibana known issue\n" + "type: known-issue\n" + "products:\n" + " - product: kibana\n" + " versions: [9.3.0]\n" + ); + + var notesByProduct = await _notesReconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); + notesByProduct.Should().ContainKey(Product); + notesByProduct[Product][Version].Should().BeEmpty(); + notesByProduct.Should().ContainKey("kibana"); + _s3.Exists(PublicBucket, ChangelogKeys.NotesIndexKey(Org, Repo, Product, Version)).Should().BeTrue(); + + var outcome = await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); + + _s3.Exists(PublicBucket, AmendNotesKey(parent)).Should().BeFalse(); + outcome.TouchedProducts.Should().Contain(Product); + _s3.Exists(PublicBucket, ChangelogKeys.NotesIndexKey(Org, Repo, Product, Version)).Should().BeTrue(); + outcome.EmptyProductIndexes.Should().Contain(new NoteAmendEmptyIndex(Product, ProductIndexKey())); + _s3.Exists(PublicBucket, "bundle/kibana/kibana-9.3.0.amend-notes.yaml").Should().BeFalse(); + } + + [Fact] + public async Task SidecarDeleteIoFailure_Throws_AndLeavesProductScopedIndex() + { + const string parent = "elasticsearch-9.3.0.yaml"; + _s3.Seed(PublicBucket, RegistryKey(), RegistryJson(parent)); + _s3.Seed(PublicBucket, BundleKey(parent), ParentBundleYaml("main/pr-100.yaml")); + var staleAmend = new Bundle + { + Products = [new BundledProduct(Product, target: Version, lifecycle: Lifecycle.Ga)], + Entries = + [ + new BundledEntry + { + File = new BundledFile { Name = "main/note-cve.yml", Checksum = "old" }, + Title = "CVE", + Type = ChangelogEntryType.Security + } + ] + }; + _s3.Seed(PublicBucket, AmendNotesKey(parent), ReleaseNotesSerialization.SerializeBundle(staleAmend)); + _s3.Seed( + PublicBucket, + ChangelogKeys.NotesIndexKey(Org, Repo, Product, Version), + /*lang=json,strict*/ + """{"schema_version":1,"product":"elasticsearch","version":"9.3.0","notes":[]}""" + ); + + var notesByProduct = await _notesReconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); + _s3.Exists(PublicBucket, ChangelogKeys.NotesIndexKey(Org, Repo, Product, Version)).Should().BeTrue(); + + var sidecarKey = AmendNotesKey(parent); + _s3.DeleteFault = + key => key == sidecarKey ? new AmazonS3Exception("unavailable") { StatusCode = HttpStatusCode.InternalServerError } : null; + + var act = async () => await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); + await act.Should().ThrowAsync(); + _s3.Exists(PublicBucket, sidecarKey).Should().BeTrue(); + _s3.Exists(PublicBucket, ChangelogKeys.NotesIndexKey(Org, Repo, Product, Version)).Should().BeTrue(); } [Fact] @@ -259,15 +393,15 @@ public async Task Idempotent_SameStateRedelivered_NoSecondPut() _s3.Seed(PublicBucket, BundleKey(parent), ParentBundleYaml("main/pr-100.yaml")); _s3.Seed(PublicBucket, NoteKey("main", "note-cve.yml"), NoteYaml); - var notesByVersion = NotesByVersion(Version, "main/note-cve.yml"); + var notesByProduct = NotesByProduct(Product, Version, "main/note-cve.yml"); // First reconcile → amend sidecar written. - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); var putsAfterFirst = _s3.Puts.Count; putsAfterFirst.Should().BeGreaterThan(0, "first pass must write the amend sidecar and the notes index"); // Second reconcile with the same state → content is identical → no additional PUTs. - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); var putsAfterSecond = _s3.Puts.Count; // The notes index re-write is idempotent too (same content, conditional PUT is a no-op). @@ -284,8 +418,8 @@ public async Task NoBundleForVersion_NoAmend_SeqRemainsZero() _s3.Seed(PublicBucket, BundleKey("elasticsearch-8.0.0.yaml"), ParentBundleYaml("main/pr-100.yaml")); _s3.Seed(PublicBucket, NoteKey("main", "note-cve.yml"), NoteYaml); - var notesByVersion = NotesByVersion(Version, "main/note-cve.yml"); - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + var notesByProduct = NotesByProduct(Product, Version, "main/note-cve.yml"); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); // No amend sidecar: no matching bundle. _s3.Puts.Should().NotContain(p => p.Key.Contains("amend-notes"), "no matching bundle → no amend possible"); @@ -297,13 +431,102 @@ public async Task NoBundleForVersion_NoAmend_SeqRemainsZero() } [Fact] - public async Task NoProductsInBundleTree_NoAmend() + public async Task NoPublishedBundle_NoAmend() { - // Bundle tree is empty (no products listed under bundle/). - // No registry.json objects exist, so ListObjectsV2 returns no common prefixes. - var notesByVersion = NotesByVersion(Version, "main/note-cve.yml"); - await _reconciler.ReconcileAsync(NotesScope(), notesByVersion, TestContext.Current.CancellationToken); + // Product is in the notes map but has no bundle registry — do not walk other products. + var notesByProduct = NotesByProduct(Product, Version, "main/note-cve.yml"); + await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); + + _s3.Puts.Should().NotContain(p => p.Key.Contains("amend-notes", StringComparison.Ordinal)); + } + + [Fact] + public async Task EceNote_DoesNotWriteOrDeleteHostedAmendNotes() + { + const string ece = "cloud-enterprise"; + const string hosted = "cloud-hosted"; + const string version = "4.2.0"; + const string eceParent = "cloud-4.2.0.yaml"; + const string hostedParent = "cloud-4.2.0.yaml"; + const string hostedSidecar = "cloud-4.2.0.amend-notes.yaml"; + + var eceNoteYaml = "title: ECE note\n" + + "type: known-issue\n" + + "products:\n" + + " - product: cloud-enterprise\n" + + " versions: [4.2.0]\n"; + + _s3.Seed( + PublicBucket, + $"bundle/{ece}/registry.json", + JsonSerializer.Serialize( + new ChangelogRegistry { Product = ece, Bundles = [new ChangelogRegistryBundle { File = eceParent, Target = version }] }, + ChangelogRegistryJsonContext.Default.ChangelogRegistry + ) + ); + _s3.Seed( + PublicBucket, + $"bundle/{hosted}/registry.json", + JsonSerializer.Serialize( + new ChangelogRegistry + { + Product = hosted, + Bundles = + [ + new ChangelogRegistryBundle { File = hostedParent, Target = version }, + new ChangelogRegistryBundle { File = hostedSidecar, Target = version } + ] + }, + ChangelogRegistryJsonContext.Default.ChangelogRegistry + ) + ); + _s3.Seed(PublicBucket, $"bundle/{ece}/{eceParent}", ParentBundleFor(ece, version, "main/pr-100.yaml")); + _s3.Seed(PublicBucket, $"bundle/{hosted}/{hostedParent}", ParentBundleFor(hosted, version, "main/pr-hosted.yaml")); - _s3.Puts.Should().NotContain(p => p.Key.Contains("amend-notes")); + var hostedAmend = new Bundle + { + Products = [new BundledProduct(hosted, target: version, lifecycle: Lifecycle.Ga)], + Entries = + [ + new BundledEntry + { + File = new BundledFile { Name = "main/note-hosted.yml", Checksum = "old" }, + Title = "Hosted note", + Type = ChangelogEntryType.KnownIssue + } + ] + }; + var hostedSidecarYaml = ReleaseNotesSerialization.SerializeBundle(hostedAmend); + _s3.Seed(PublicBucket, $"bundle/{hosted}/{hostedSidecar}", hostedSidecarYaml); + _s3.Seed(PublicBucket, NoteKey("main", "note-ece.yml"), eceNoteYaml); + + var notesByProduct = NotesByProduct(ece, version, "main/note-ece.yml"); + var outcome = await _reconciler.ReconcileAsync(NotesScope(), notesByProduct, TestContext.Current.CancellationToken); + + outcome.TouchedProducts.Should().Equal(ece); + outcome.EmptyProductIndexes.Should().BeEmpty(); + _s3.Exists(PublicBucket, $"bundle/{ece}/cloud-4.2.0.amend-notes.yaml").Should().BeTrue(); + _s3.ContentOf(PublicBucket, $"bundle/{hosted}/{hostedSidecar}").Should().Be(hostedSidecarYaml); + _s3.Deletes.Should().NotContain(d => d.Key.Contains($"bundle/{hosted}/", StringComparison.Ordinal)); + } + + private static string ParentBundleFor(string product, string version, params string[] entryFileNames) + { + var bundle = new Bundle + { + Products = [new BundledProduct(product, target: version, lifecycle: Lifecycle.Ga)], + Entries = + [ + .. entryFileNames.Select( + n => new BundledEntry + { + File = new BundledFile { Name = n, Checksum = "abc123" }, + Title = $"Entry for {n}", + Type = ChangelogEntryType.BugFix + } + ) + ] + }; + return ReleaseNotesSerialization.SerializeBundle(bundle); } } diff --git a/tests/Elastic.Changelog.Tests/Reconciliation/NotesIndexReconcilerTests.cs b/tests/Elastic.Changelog.Tests/Reconciliation/NotesIndexReconcilerTests.cs index e2147b80fa..06d2f68e31 100644 --- a/tests/Elastic.Changelog.Tests/Reconciliation/NotesIndexReconcilerTests.cs +++ b/tests/Elastic.Changelog.Tests/Reconciliation/NotesIndexReconcilerTests.cs @@ -2,7 +2,9 @@ // 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 Amazon.S3; using AwesomeAssertions; using Elastic.Changelog.Reconciliation; using Elastic.Documentation.Configuration.ReleaseNotes; @@ -284,33 +286,49 @@ public async Task ReconcileRepo_TwoProductsSameVersion_ProductIndexesAreIsolated } [Fact] - public async Task ReconcileRepo_ProductScopedStale_DeletedWithoutDroppingLegacyVersion() + public async Task ReconcileRepo_ProductScopedStale_ReturnedAsEmptyBucketWithoutDeleting() { + var kibanaKey = ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "kibana", "9.0.0"); _s3.Seed( PublicBucket, - ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "kibana", "9.0.0"), + kibanaKey, /*lang=json,strict*/ """{"schema_version":1,"product":"kibana","version":"9.0.0","notes":[]}""" ); SeedNote("main", "note-slow-rollover.yml", NoteYaml); - await _reconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); + var map = await _reconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); _s3.Exists(PublicBucket, ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "9.0.0")).Should().BeTrue(); _s3.Exists(PublicBucket, ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "elasticsearch", "9.0.0")).Should().BeTrue(); - _s3 - .Deletes - .Should() - .ContainSingle() - .Which - .Key - .Should() - .Be(ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "kibana", "9.0.0")); + _s3.Exists(PublicBucket, kibanaKey).Should().BeTrue(); + _s3.Deletes.Should().NotContain(d => d.Key == kibanaKey); + map.Should().ContainKey("kibana"); + map["kibana"].Should().ContainKey("9.0.0"); + map["kibana"]["9.0.0"].Should().BeEmpty(); + map["elasticsearch"]["9.0.0"].Should().ContainSingle(e => e.Path == "main/note-slow-rollover.yml"); + } + + [Fact] + public async Task ReconcileRepo_NoNotes_LegacyIndexWithoutProduct_DoesNotInventVanishedProduct() + { + _s3.Seed( + PublicBucket, + ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "9.0.0"), + /*lang=json,strict*/ + """{"schema_version":1,"notes":[]}""" + ); + + var map = await _reconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); + + map.Should().BeEmpty(); + _s3.Deletes.Should().ContainSingle().Which.Key.Should().Be(ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "9.0.0")); } [Fact] - public async Task ReconcileRepo_NoNotes_DeletesProductScopedAndLegacyIndexes() + public async Task ReconcileRepo_NoNotes_DeletesLegacyOnly_KeepsProductScopedForAmend() { + var productKey = ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "elasticsearch", "9.0.0"); _s3.Seed( PublicBucket, ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "9.0.0"), @@ -319,22 +337,58 @@ public async Task ReconcileRepo_NoNotes_DeletesProductScopedAndLegacyIndexes() ); _s3.Seed( PublicBucket, - ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "elasticsearch", "9.0.0"), + productKey, /*lang=json,strict*/ """{"schema_version":1,"product":"elasticsearch","version":"9.0.0","notes":[]}""" ); _s3.Seed(PublicBucket, "changelog/elastic/elasticsearch/main/12345.yaml", "title: PR entry"); - await _reconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); + var map = await _reconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); _s3.Puts.Should().BeEmpty(); - _s3 - .Deletes - .Select(d => d.Key) - .Should() - .BeEquivalentTo([ - ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "9.0.0"), - ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "elasticsearch", "9.0.0") - ]); + _s3.Deletes.Should().ContainSingle().Which.Key.Should().Be(ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "9.0.0")); + _s3.Exists(PublicBucket, productKey).Should().BeTrue(); + map.Should().ContainKey("elasticsearch"); + map["elasticsearch"]["9.0.0"].Should().BeEmpty(); + map.Should().NotContainKey("9.0.0"); + } + + [Fact] + public async Task ReconcileRepo_UnparseableProductScopedIndex_IsLeftInPlace() + { + var kibanaKey = ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "kibana", "9.0.0"); + _s3.Seed(PublicBucket, kibanaKey, "{not-json"); + SeedNote("main", "note-slow-rollover.yml", NoteYaml); + + var map = await _reconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); + + _s3.Exists(PublicBucket, kibanaKey).Should().BeTrue(); + _s3.Deletes.Should().NotContain(d => d.Key == kibanaKey); + map.Should().NotContainKey("kibana"); + map["elasticsearch"]["9.0.0"].Should().ContainSingle(e => e.Path == "main/note-slow-rollover.yml"); + } + + [Fact] + public async Task ReconcileRepo_ProductScopedIndexGetFailure_IsLeftInPlace() + { + var kibanaKey = ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", "kibana", "9.0.0"); + _s3.Seed( + PublicBucket, + kibanaKey, + /*lang=json,strict*/ + """{"schema_version":1,"product":"kibana","version":"9.0.0","notes":[]}""" + ); + SeedNote("main", "note-slow-rollover.yml", NoteYaml); + _s3.AfterGet = (key, _) => + { + if (key == kibanaKey) + throw new AmazonS3Exception("unavailable") { StatusCode = HttpStatusCode.InternalServerError }; + }; + + var map = await _reconciler.ReconcileRepoAsync(NotesScope(), TestContext.Current.CancellationToken); + + _s3.Exists(PublicBucket, kibanaKey).Should().BeTrue(); + _s3.Deletes.Should().NotContain(d => d.Key == kibanaKey); + map.Should().NotContainKey("kibana"); } } diff --git a/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs b/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs index 7dcc42fa01..f6e8ecf4da 100644 --- a/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs +++ b/tests/Elastic.Changelog.Tests/Scrubbing/ScrubberProcessorTests.cs @@ -2,7 +2,9 @@ // 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 Amazon.S3; using AwesomeAssertions; using Elastic.Changelog.Reconciliation; using Elastic.Changelog.Scrubbing; @@ -517,6 +519,133 @@ public async Task Process_NoteFile_ScrubbedAndNotesReconcileTriggered() _s3.Exists(PublicBucket, "changelog/elastic/elasticsearch/notes-elasticsearch-9.0.0.json").Should().BeTrue(); } + [Fact] + public async Task Process_NoteFile_ListsAmendNotesOnThatProductOnly() + { + _ = A.CallTo(() => _scrubber.ScrubAsync(A._, A._, A._)).ReturnsLazily( + (string _, string content, Cancel _) => Task.FromResult(new ScrubResult { Content = content }) + ); + + const string ece = "cloud-enterprise"; + const string hosted = "cloud-hosted"; + const string version = "4.2.0"; + const string parent = "cloud-4.2.0.yaml"; + const string hostedSidecar = "cloud-4.2.0.amend-notes.yaml"; + + _s3.Seed(PublicBucket, $"bundle/{ece}/{parent}", ProductParentBundle(ece, version, "main/pr-100.yaml")); + _s3.Seed(PublicBucket, $"bundle/{hosted}/{parent}", ProductParentBundle(hosted, version, "main/pr-hosted.yaml")); + _s3.Seed(PublicBucket, $"bundle/{ece}/registry.json", ProductRegistryJson(ece, version, parent)); + _s3.Seed(PublicBucket, $"bundle/{hosted}/registry.json", ProductRegistryJson(hosted, version, parent, hostedSidecar)); + var hostedSidecarYaml = ProductParentBundle(hosted, version, "main/note-hosted.yml"); + _s3.Seed(PublicBucket, $"bundle/{hosted}/{hostedSidecar}", hostedSidecarYaml); + + const string noteYaml = + """ + title: ECE known issue + type: known-issue + products: + - product: cloud-enterprise + versions: [4.2.0] + """; + _s3.Seed(PrivateBucket, "changelog/elastic/cloud/main/note-ece.yml", noteYaml); + + var failed = await _processor.ProcessAsync([Message("ObjectCreated:Put", "changelog/elastic/cloud/main/note-ece.yml")], Ctx); + + failed.Should().BeEmpty(); + _s3.Exists(PublicBucket, $"bundle/{ece}/cloud-4.2.0.amend-notes.yaml").Should().BeTrue(); + _s3.ContentOf(PublicBucket, $"bundle/{hosted}/{hostedSidecar}").Should().Be(hostedSidecarYaml); + + var eceRegistry = + JsonSerializer.Deserialize( + _s3.ContentOf(PublicBucket, $"bundle/{ece}/registry.json"), + ChangelogRegistryJsonContext.Default.ChangelogRegistry + )!; + eceRegistry.Bundles.Select(b => b.File).Should().BeEquivalentTo([parent, "cloud-4.2.0.amend-notes.yaml"]); + + var hostedRegistry = + JsonSerializer.Deserialize( + _s3.ContentOf(PublicBucket, $"bundle/{hosted}/registry.json"), + ChangelogRegistryJsonContext.Default.ChangelogRegistry + )!; + hostedRegistry.Bundles.Select(b => b.File).Should().BeEquivalentTo([parent, hostedSidecar]); + _metrics.GroupReconciles.Should().Be(1); + } + + [Fact] + public async Task Process_LastNoteRemoved_RegistryWriteFailure_LeavesProductScopedIndex_ThenRetryDeletes() + { + _ = A.CallTo(() => _scrubber.ScrubAsync(A._, A._, A._)).ReturnsLazily( + (string _, string content, Cancel _) => Task.FromResult(new ScrubResult { Content = content }) + ); + + const string product = "elasticsearch"; + const string version = "9.0.0"; + const string parent = "elasticsearch-9.0.0.yaml"; + const string sidecar = "elasticsearch-9.0.0.amend-notes.yaml"; + const string noteKey = "changelog/elastic/elasticsearch/main/note-late.yml"; + var productIndex = ChangelogKeys.NotesIndexKey("elastic", "elasticsearch", product, version); + var registryKey = $"bundle/{product}/registry.json"; + + _s3.Seed(PublicBucket, $"bundle/{product}/{parent}", ProductParentBundle(product, version, "main/pr-100.yaml")); + _s3.Seed(PublicBucket, registryKey, ProductRegistryJson(product, version, parent, sidecar)); + _s3.Seed(PublicBucket, $"bundle/{product}/{sidecar}", ProductParentBundle(product, version, "main/note-late.yml")); + _s3.Seed( + PublicBucket, + productIndex, + /*lang=json,strict*/ + """{"schema_version":1,"product":"elasticsearch","version":"9.0.0","notes":[{"path":"main/note-late.yml","bundle_seq":2}]}""" + ); + _s3.Seed( + PublicBucket, + "changelog/elastic/elasticsearch/notes-9.0.0.json", + /*lang=json,strict*/ + """{"schema_version":1,"notes":[{"path":"main/note-late.yml","bundle_seq":2}]}""" + ); + + _s3.PutFault = + key => key == registryKey ? new AmazonS3Exception("unavailable") { StatusCode = HttpStatusCode.InternalServerError } : null; + + var failed = await _processor.ProcessAsync([Message("ObjectRemoved:Delete", noteKey)], Ctx); + failed.Should().NotBeEmpty(); + _s3.Exists(PublicBucket, $"bundle/{product}/{sidecar}").Should().BeFalse(); + _s3.Exists(PublicBucket, productIndex).Should().BeTrue(); + + _s3.PutFault = null; + var failedRetry = await _processor.ProcessAsync([Message("ObjectRemoved:Delete", noteKey)], Ctx); + failedRetry.Should().BeEmpty(); + _s3.Exists(PublicBucket, productIndex).Should().BeFalse(); + PublicManifest(registryKey).Bundles.Select(b => b.File).Should().Equal(parent); + } + + private static string ProductRegistryJson(string product, string version, params string[] files) + { + var bundles = files.Select(f => new ChangelogRegistryBundle { File = f, Target = version }).ToList(); + return JsonSerializer.Serialize( + new ChangelogRegistry { Product = product, Bundles = bundles }, + ChangelogRegistryJsonContext.Default.ChangelogRegistry + ); + } + + private static string ProductParentBundle(string product, string version, params string[] entryFileNames) + { + var bundle = new Bundle + { + Products = [new BundledProduct(product, target: version, lifecycle: Lifecycle.Ga)], + Entries = + [ + .. entryFileNames.Select( + n => new BundledEntry + { + File = new BundledFile { Name = n, Checksum = "abc123" }, + Title = $"Entry for {n}", + Type = ChangelogEntryType.BugFix + } + ) + ] + }; + return ReleaseNotesSerialization.SerializeBundle(bundle); + } + [Fact] public async Task Process_PassThroughMarker_DoesNotOverwriteExistingCanonicalContent() {