From c0413cbd5b5bf8d0a4c2f0ce826c30dbc1bb561e Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Wed, 24 Jun 2026 13:28:57 -0300 Subject: [PATCH 1/6] changelog: restructure S3 layout to artifact-root (Option AD) Move changelog/bundle S3 keys from product-first to artifact-root: bundles are product-scoped under bundle/{product}/{file}; changelog entries are stored once per authoring repo under changelog/{repo}/{file}. This decouples repository concerns (entries) from product concerns (bundles) and removes the developer- foreknowledge requirement for entry distribution. - upload: entries -> changelog/{repo}/{file} (repo via --repo > bundle.repo > git remote); bundles -> bundle/{product}/{file} - registries: bundle/{product}/registry.json and changelog/{repo}/registry.json; scope-aware grouping and RegistryKey.IsRegistry validation - CDN consumers + bundling: repo-scoped entry sourcing, decoupled gate, ResolveCdnBundleUrl -> bundle/{product}/{file} - scrubber Lambda: detect bundles by the bundle/ key prefix - CLI: add --repo/--owner to `changelog upload`; regenerate cli-schema.json - tests + docs updated; new cloud-shaped profile fixture Refs elastic/docs-eng-team#413 Co-authored-by: Cursor --- docs/cli-schema.json | 18 +- docs/cli/changelog/cmd-upload.md | 34 +-- docs/contribute/configure-changelogs-ref.md | 16 +- docs/development/changelog-bundle-registry.md | 65 +++--- .../ReleaseNotes/CdnChangelogEntryFetcher.cs | 40 ++-- .../ReleaseNotes/CdnChangelogFetcher.cs | 8 +- .../ReleaseNotes/ChangelogRegistry.cs | 14 +- .../Directives/Changelog/ChangelogBlock.cs | 2 +- .../docs-lambda-changelog-scrubber/Program.cs | 5 +- .../docs-lambda-changelog-scrubber/README.md | 6 +- .../Bundling/ChangelogBundlingService.cs | 183 +++++----------- .../Uploading/ChangelogUploadService.cs | 86 ++++---- .../Elastic.Changelog/Uploading/Registry.cs | 12 +- .../Uploading/RegistryBuilder.cs | 48 +++-- .../Uploading/RegistryKey.cs | 44 ++-- .../docs-builder/Commands/ChangelogCommand.cs | 52 ++++- .../Changelogs/BundleCdnSourcingTests.cs | 44 ++-- .../Changelogs/BundlePlanTests.cs | 22 +- .../Changelogs/CloudProfileFixtureTests.cs | 196 ++++++++++++++++++ .../Changelogs/LinkAllowlistSanitizerTests.cs | 2 +- .../Uploading/ChangelogUploadServiceTests.cs | 191 ++++++++++------- .../Uploading/RegistryBuilderTests.cs | 44 ++-- .../Uploading/RegistryKeyTests.cs | 52 +++-- .../CdnChangelogEntryFetcherTests.cs | 12 +- .../ReleaseNotes/CdnChangelogFetcherTests.cs | 4 + 25 files changed, 734 insertions(+), 466 deletions(-) create mode 100644 tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs diff --git a/docs/cli-schema.json b/docs/cli-schema.json index fe660eb2e..5aa8dc608 100644 --- a/docs/cli-schema.json +++ b/docs/cli-schema.json @@ -1,7 +1,7 @@ { "schemaVersion": 1, "name": "docs-builder", - "version": "0", + "version": "1", "reservedMetaCommands": [ "__complete", "__completion", @@ -3898,7 +3898,7 @@ ], "name": "upload", "summary": "Upload changelog entries or bundle artifacts to S3 or Elasticsearch.", - "notes": "Uses content-hash\u2013based incremental transfer \u2014 only changed files are uploaded.", + "notes": "Uses content-hash\u2013based incremental transfer \u2014 only changed files are uploaded.\n\n\nChangelog entries are uploaded once under changelog/{repo}/{file}, keyed by the authoring\nrepository (resolved from --repo, then bundle.repo in changelog.yml, then the git\nremote origin). Bundles are uploaded under bundle/{product}/{file}, product-scoped from\nthe bundle YAML, and do not require a repo.", "usage": "docs-builder changelog upload --artifact-type \u003Cstring\u003E --target \u003Cstring\u003E [options]", "examples": [], "parameters": [ @@ -3957,6 +3957,20 @@ } ] }, + { + "role": "flag", + "name": "repo", + "type": "string", + "required": false, + "summary": "GitHub repository name used to scope uploaded changelog entry keys (changelog/{repo}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads." + }, + { + "role": "flag", + "name": "owner", + "type": "string", + "required": false, + "summary": "GitHub repository owner, accepted for parity with other changelog commands. Falls back to bundle.owner in changelog.yml, then the git remote origin. Not part of the S3 key." + }, { "role": "flag", "name": "log-level", diff --git a/docs/cli/changelog/cmd-upload.md b/docs/cli/changelog/cmd-upload.md index 73af2d091..2b9be2dc8 100644 --- a/docs/cli/changelog/cmd-upload.md +++ b/docs/cli/changelog/cmd-upload.md @@ -1,6 +1,6 @@ ## Description -Upload changelog entries or bundle artifacts to S3 or Elasticsearch. The command discovers `.yaml` and `.yml` files in a local directory, maps each file to one or more product-scoped S3 keys, and uploads only files whose content hash changed since the last run. +Upload changelog entries or bundle artifacts to S3 or Elasticsearch. The command discovers `.yaml` and `.yml` files in a local directory and uploads only files whose content hash changed since the last run. Changelog entries are uploaded once under `changelog/{repo}/{file}`, keyed by the authoring repository; bundles are uploaded under `bundle/{product}/{file}`, product-scoped from the bundle YAML. To create bundles first, use [](/cli/changelog/bundle.md). For the end-to-end workflow, see [](/contribute/bundle-changelogs.md). @@ -42,10 +42,8 @@ Your IAM policy must allow these S3 actions on the target bucket: You can scope object-level permissions to the key prefixes the command writes: -- `{product}/bundle/*` -- `{product}/changelog/*` -- `{product}/registry.json` -- `{product}/changelog/registry.json` +- `bundle/*` (bundle YAML and `bundle/{product}/registry.json`) +- `changelog/*` (entry YAML and `changelog/{repo}/registry.json`) #### Local development @@ -79,10 +77,10 @@ Use `--artifact-type` to choose what to upload: | `bundle` | Consolidated bundle YAML files | `bundle.output_directory` from `changelog.yml`, or `docs/releases` | | `changelog` | Individual changelog entry YAML files | `bundle.directory` from `changelog.yml`, or `docs/changelog` | -Each file is uploaded once per product listed in its YAML: +Keying differs by artifact type: -- Changelog entries use the `products[].product` field. -- Bundles use the `products[].product` field (product ID). +- **Changelog entries** are uploaded **once** under the authoring repository, regardless of how many products they list (or none). The repo is resolved from `--repo`, then `bundle.repo` in `changelog.yml`, then the git remote origin. +- **Bundles** are uploaded once per product listed in the bundle's `products[].product` field (a bundle that declares multiple products is written under each product prefix). ## Upload targets @@ -98,19 +96,21 @@ Use `--target` to choose the destination: For each discovered file, the command writes to: ```text -s3://{bucket}/{product}/changelog/{filename} # --artifact-type changelog -s3://{bucket}/{product}/bundle/{filename} # --artifact-type bundle +s3://{bucket}/changelog/{repo}/{filename} # --artifact-type changelog +s3://{bucket}/bundle/{product}/{filename} # --artifact-type bundle ``` -A changelog or bundle that applies to multiple products is uploaded to multiple keys — one per product. +Changelog entries are written once under the authoring repo. A bundle that applies to multiple products is uploaded to multiple keys — one per product. -After a successful upload, the command refreshes per-product `registry.json` manifests at: +After a successful upload, the command refreshes the relevant `registry.json` manifest: ```text -s3://{bucket}/{product}/registry.json # bundle uploads -s3://{bucket}/{product}/changelog/registry.json # changelog uploads +s3://{bucket}/changelog/{repo}/registry.json # changelog uploads +s3://{bucket}/bundle/{product}/registry.json # bundle uploads ``` +When several repositories publish bundles for the same shared product (for example `cloud-serverless`), use a `{repo}-{dateOrVersion}.yaml` bundle filename convention so they don't overwrite each other under `bundle/{product}/`. + The registry refresh is best-effort: upload failures block the run, but a stale manifest does not fail an otherwise successful upload. :::{note} @@ -142,13 +142,15 @@ docs-builder changelog upload \ ### Upload changelog entries to S3 -Upload individual changelog YAML files from the default changelog directory (`docs/changelog`): +Upload individual changelog YAML files from the default changelog directory (`docs/changelog`). Entries are written to `changelog/{repo}/...`; pass `--repo` (and optionally `--owner`) when the authoring repo can't be inferred from `bundle.repo` or the git remote: ```sh docs-builder changelog upload \ --artifact-type changelog \ --target s3 \ - --s3-bucket-name my-changelog-bundles + --s3-bucket-name my-changelog-bundles \ + --repo my-repo \ + --owner elastic ``` ### Override the source directory diff --git a/docs/contribute/configure-changelogs-ref.md b/docs/contribute/configure-changelogs-ref.md index edc321364..b0e36da8f 100644 --- a/docs/contribute/configure-changelogs-ref.md +++ b/docs/contribute/configure-changelogs-ref.md @@ -51,7 +51,7 @@ These settings are relevant to one or all of the `changelog bundle`, `changelog | `bundle.output_directory` | Output directory for bundled files (default: `docs/releases`). | | `bundle.owner` | Default GitHub repository owner (for example, `elastic`). | | `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.repo` | Default GitHub repository name (for example, `elasticsearch`). Used by the `{changelog}` directive to generate correct PR and issue links, and to scope uploaded changelog-entry keys (`changelog/{repo}/...`) and CDN entry sourcing. Only needed when the product ID doesn't match the GitHub repository name (or to override the git remote). | | `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). | @@ -66,20 +66,16 @@ When `bundle.link_allow_repos` is omitted, no link filtering occurs. ### 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`. +`changelog bundle` reads the individual changelog entries it aggregates either from the local folder or from the public CDN. Entries are stored on the CDN per **authoring repository** (`changelog/{repo}/...`), not per product, so CDN sourcing keys off the resolvable authoring repo rather than the bundle's target products. -```yaml -# docset.yml -release_notes: - - product: elasticsearch -``` +The authoring repo is resolved with the same precedence as `changelog upload`: `--repo` > `bundle.repo` in `changelog.yml` > the git remote origin. 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. +- **Local folder.** Used when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when the authoring repo cannot be resolved. The folder must contain the changelog files. +- **CDN (default when a repo resolves).** Used when the authoring repo resolves, local sourcing is not forced, and a CDN base URL is configured (`DOCS_BUILDER_CHANGELOG_CDN`, defaulting to the public distribution). The command fetches `changelog/{repo}/registry.json` and the entries it lists, then applies the bundle's own product/PR/issue filters to the downloaded set. -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. +Because entries are repo-scoped, one repository can produce a bundle for a shared product (for example, `cloud-serverless`) while sourcing its own entries from `changelog/{repo}/`, without that product appearing in the repository's `docset.yml`. The `{changelog}` directive's `:cdn:` mode still consumes product-scoped *bundles*, so a repository that also renders its own release notes declares each product under `release_notes` as before. ### Bundle descriptions [bundle-descriptions] diff --git a/docs/development/changelog-bundle-registry.md b/docs/development/changelog-bundle-registry.md index 26b6b67c4..a8e3befce 100644 --- a/docs/development/changelog-bundle-registry.md +++ b/docs/development/changelog-bundle-registry.md @@ -34,8 +34,8 @@ copies, no cross-repo file syncing. │ Client CI │ --artifact-type │ Private bundles │ ───────────────────▶ │ Changelog scrubber │ │ (docs-actions)│ bundle ───────────▶ │ S3 bucket │ │ Lambda │ └──────────────┘ │ │ └─────────┬─────────┘ - │ │ {product}/bundle/*.yaml │ scrub + copy - │ also refreshes │ {product}/registry.json │ (pass-through for + │ │ bundle/{product}/*.yaml │ scrub + copy + │ also refreshes │ bundle/{product}/registry.json │ (pass-through for └──────────────────────────────▶ │ │ registry.json) └────────────────────┘ ▼ ┌───────────────────┐ @@ -46,24 +46,27 @@ copies, no cross-repo file syncing. 1. **Producer** — `changelog upload --artifact-type bundle --target s3` (invoked by the docs-actions changelog upload workflow) uploads each bundle to - `{product}/bundle/{file}` in the **private** bucket, then refreshes - `{product}/registry.json` for every product the run touched. + `bundle/{product}/{file}` in the **private** bucket, then refreshes + `bundle/{product}/registry.json` for every product the run touched. The companion + `--artifact-type changelog` upload writes individual entries to `changelog/{repo}/{file}` + (one copy, keyed by the authoring repo) and refreshes `changelog/{repo}/registry.json`. 2. **Scrubber Lambda** — triggered by `s3:ObjectCreated` on the private bucket, it scrubs - private repository references out of bundle YAML and writes the sanitized copy to the - **public** bucket. The `registry.json` object is copied through **verbatim**. + private repository references out of bundle and entry YAML and writes the sanitized copy to + the **public** bucket. The `registry.json` object is copied through **verbatim**. 3. **Consumer** — for each product declared under `release_notes` in `docset.yml`, docs-builder - reads `{product}/registry.json` from the CDN at build startup and fetches each listed bundle; - the `{changelog}` directive in `cdn:` mode then renders from the prefetched result. + reads `bundle/{product}/registry.json` from the CDN at build startup and fetches each listed + bundle; the `{changelog}` directive in `cdn:` mode then renders from the prefetched result. ### Why a registry instead of an S3 listing The public surface is a CDN (CloudFront) in front of S3. CloudFront does not expose bucket -listing, so the consumer cannot enumerate `{product}/bundle/`. The registry is a stable, +listing, so the consumer cannot enumerate `bundle/{product}/`. The registry is a stable, cacheable manifest at a predictable key that lists exactly which bundles exist for a product. ## `registry.json` format -Stored at `{product}/registry.json`. Serialized with `snake_case` keys. +Stored at `bundle/{product}/registry.json` (bundle index) or `changelog/{repo}/registry.json` +(changelog-entry index). Serialized with `snake_case` keys. ```json { @@ -80,9 +83,9 @@ Stored at `{product}/registry.json`. Serialized with `snake_case` keys. | Field | Meaning | |---|---| | `schema_version` | Bumped when consumers must change their parser. | -| `product` | Product identifier; matches the first S3 key segment. | +| `product` | Grouping identifier; the second S3 key segment — the product for a bundle index (`bundle/{product}/…`) or the authoring repo for a changelog-entry index (`changelog/{repo}/…`). | | `generated_at` | UTC timestamp of the last regeneration. | -| `bundles[].file` | Bundle file name, resolved at `{product}/bundle/{file}`. | +| `bundles[].file` | Bundle file name, resolved at `bundle/{product}/{file}` (or entry file at `changelog/{repo}/{file}` for the entry index). | | `bundles[].target` | Target version/date from the bundle's declaration of **this** product (may be null). | | `bundles[].etag` | See the ETag caveat below. | @@ -105,7 +108,8 @@ reads of the same bucket). The refresh runs inside `ChangelogUploadService` after a successful **bundle** upload (it is skipped for `--artifact-type changelog`). `RegistryBuilder`: -- Groups the run's upload targets by product (from the `{product}/bundle/{file}` key). +- Groups the run's upload targets by the second key segment — product for bundles + (`bundle/{product}/{file}`), repo for entries (`changelog/{repo}/{file}`). - For each product, derives one `registry` entry per bundle (file name, that product's target, locally-computed S3 ETag). - Reads the existing manifest from S3, merges by file name (re-uploads replace their entry; @@ -150,8 +154,9 @@ for the producer**: covering the registry `CopyObject` pass-through and the `ObjectRemoved` delete. - A CloudFront cache policy tuned for the manifest already exists (default TTL 1h, min 60s). -The scrubber only passes through keys accepted by `RegistryKey.IsRegistry` (a single -`{product}/registry.json` segment), so arbitrary JSON cannot reach the public surface. +The scrubber only passes through keys accepted by `RegistryKey.IsRegistry` +(`bundle/{product}/registry.json` or `changelog/{repo}/registry.json`, each a single middle +segment), so arbitrary JSON cannot reach the public surface. **No new docs-actions workflow logic is required** for the producer either: the refresh is a side-effect of the existing `changelog upload` step; docs-actions only needs a docs-builder @@ -166,21 +171,23 @@ 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) +## `changelog bundle` entry sourcing (repo 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`). +entries from the local folder or fetch the **authoring repo's** published entries from the CDN +(`changelog/{repo}/registry.json` → `changelog/{repo}/{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`: +Under the artifact-root layout, entries are repo-scoped — not product-scoped — so CDN entry +sourcing keys off the resolvable authoring repo (the same precedence as upload: +`--repo` > `bundle.repo` > git remote), **not** off the bundle's target products. This is what lets +one repo (for example `kibana`) produce a bundle for a shared product (for example `cloud-serverless`) +while sourcing its own entries from `changelog/kibana/`, without that product appearing in the repo's +own `docset.yml`. 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/`). +- **Local folder** when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when + the authoring repo cannot be resolved. +- **CDN** when the authoring repo resolves, local sourcing is not forced, and a CDN base is + configured (`DOCS_BUILDER_CHANGELOG_CDN`, defaulting to the public distribution). 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 @@ -239,8 +246,8 @@ notes use `NoopReleaseNotesResolver`. ### Fetch flow -1. `GET {cdnBase}/{product}/registry.json`. -2. Parse it; for each `bundles[].file`, `GET {cdnBase}/{product}/bundle/{file}`. +1. `GET {cdnBase}/bundle/{product}/registry.json`. +2. Parse it; for each `bundles[].file`, `GET {cdnBase}/bundle/{product}/{file}`. 3. Feed the downloaded YAML into the existing `BundleLoader` → `MergeBundlesByTarget` → render pipeline. **Rendering is unchanged**; only the source of the bundle bytes differs. @@ -270,7 +277,7 @@ logic still applies via `assembler.yml`, exactly as for local bundles. prefetched bundles at render time. - **Security.** The base URL is trusted configuration; the product and registry-supplied bundle file names are validated to single path segments so neither can traverse outside - `{product}/bundle/`. + `bundle/{product}/`. ### Follow-ups (not yet implemented) [implementation-notes] diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs index 30fe0fd8e..f4e48f68f 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs @@ -14,10 +14,10 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; 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 +/// Fetches the individual (scrubbed) changelog entries for a single authoring repo from the public +/// CDN, for the changelog bundle command when sourcing entries from S3 rather than a local +/// folder. It reads {base}/changelog/{repo}/registry.json to enumerate entries and downloads +/// each {base}/changelog/{repo}/{file} as raw YAML; the bundle command then applies its usual /// filter (products / prs / issues) to the downloaded set. /// /// @@ -92,19 +92,19 @@ public CdnChangelogEntryFetcher( } /// - /// Downloads the changelog entries for from the CDN at + /// Downloads the changelog entries for the authoring 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 async Task> FetchAsync( Uri baseUri, - string product, + string repo, Action emitError, Action emitWarning, Cancel ctx) { - var registryUri = Combine(baseUri, product, "changelog", "registry.json"); + var registryUri = Combine(baseUri, "changelog", repo, "registry.json"); ChangelogRegistry? registry; try @@ -113,20 +113,20 @@ public async Task> FetchAsync( } catch (Exception ex) when (ex is not OperationCanceledException) { - emitError($"Could not fetch changelog entry registry for product '{product}' from {registryUri}: {ex.Message}"); + emitError($"Could not fetch changelog entry registry for repo '{repo}' from {registryUri}: {ex.Message}"); return []; } if (registry is null) { - emitError($"Changelog entry registry for product '{product}' at {registryUri} was empty or unparseable."); + emitError($"Changelog entry registry for repo '{repo}' 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."); + $"Changelog entry registry for repo '{repo}' uses schema version {registry.SchemaVersion}, but this build only understands version {SupportedSchemaVersion}. Update docs-builder."); return []; } @@ -138,12 +138,12 @@ public async Task> FetchAsync( var fileName = entry.File; if (string.IsNullOrWhiteSpace(fileName) || !IsSafeFileName(fileName)) { - emitWarning($"Changelog entry registry for '{product}' lists an invalid file name '{fileName}'; skipping."); + emitWarning($"Changelog entry registry for '{repo}' lists an invalid file name '{fileName}'; skipping."); continue; } - var entryUri = Combine(baseUri, product, "changelog", fileName); - var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, product, ctx).ConfigureAwait(false); + var entryUri = Combine(baseUri, "changelog", repo, fileName); + var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, repo, ctx).ConfigureAwait(false); if (fetched) { entries.Add(new CdnChangelogEntry(fileName, content)); @@ -154,12 +154,12 @@ public async Task> FetchAsync( // 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}. " + + $"Changelog entry '{fileName}' for repo '{repo}' 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); + _logger.LogInformation("Fetched {Count} changelog entry(ies) for repo {Repo} from {BaseUri}", entries.Count, repo, baseUri); return entries; } @@ -168,7 +168,7 @@ public async Task> FetchAsync( /// 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 async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string product, Cancel ctx) + private async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string repo, Cancel ctx) { string? lastError = null; @@ -179,7 +179,7 @@ public async Task> FetchAsync( { 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); + _logger.LogInformation("Fetched changelog entry '{File}' for {Repo} on attempt {Attempt}/{Max}", fileName, repo, attempt, _maxAttempts); return (true, content, null); } catch (Exception ex) when (ex is not OperationCanceledException) @@ -190,8 +190,8 @@ public async Task> FetchAsync( 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); + "Changelog entry '{File}' for {Repo} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", + fileName, repo, attempt, _maxAttempts, ex.Message, delay); await _sleep(delay, ctx).ConfigureAwait(false); } } @@ -250,7 +250,7 @@ private static Uri Combine(Uri baseUri, params string[] segments) /// /// 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}). + /// must be a single path segment (the producer always writes changelog/{repo}/{file}). /// private static bool IsSafeFileName(string fileName) => !fileName.Contains('/', StringComparison.Ordinal) diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs index 21d33b3e1..f19ddbf23 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs @@ -12,8 +12,8 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; /// /// Fetches changelog bundles for a single product from the public CDN. It reads -/// {base}/{product}/registry.json to enumerate bundles, downloads each -/// {base}/{product}/bundle/{file}, and parses them via +/// {base}/bundle/{product}/registry.json to enumerate bundles, downloads each +/// {base}/bundle/{product}/{file}, and parses them via /// . /// /// @@ -93,7 +93,7 @@ public async Task> FetchAsync( Action emitWarning, Cancel ctx) { - var registryUri = Combine(baseUri, product, "registry.json"); + var registryUri = Combine(baseUri, "bundle", product, "registry.json"); ChangelogRegistry? registry; try @@ -164,7 +164,7 @@ public async Task> FetchAsync( continue; } - var bundleUri = Combine(baseUri, product, "bundle", fileName); + var bundleUri = Combine(baseUri, "bundle", product, fileName); try { contents.Add((fileName, await FetchTextAsync(bundleUri, ctx).ConfigureAwait(false))); diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs index cccbf489b..3ded1cd8d 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs @@ -7,18 +7,18 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; /// -/// Consumer-side view of the per-product {product}/registry.json manifest published -/// alongside scrubbed changelog bundles. Mirrors the producer's shape (see the changelog upload -/// service) but is intentionally lenient: only the fields the changelog directive needs to -/// enumerate bundles are declared, and nothing is required so a partially-written or -/// future-versioned manifest still deserializes. +/// Consumer-side view of the bundle/{product}/registry.json manifest (or the +/// changelog/{repo}/registry.json entry index) published alongside scrubbed changelog content. +/// Mirrors the producer's shape (see the changelog upload service) but is intentionally lenient: only +/// the fields the changelog directive needs to enumerate bundles are declared, and nothing is +/// required so a partially-written or future-versioned manifest still deserializes. /// public sealed record ChangelogRegistry { /// Manifest schema version. A higher major than the consumer understands is rejected upstream. public int SchemaVersion { get; init; } = 1; - /// Product identifier the manifest belongs to (matches the first S3 key segment). + /// Grouping identifier the manifest belongs to (the second S3 key segment: product for a bundle index, repo for a changelog-entry index). public string? Product { get; init; } /// Bundles known for this product, newest first as written by the producer. @@ -28,7 +28,7 @@ public sealed record ChangelogRegistry /// One entry in . public sealed record ChangelogRegistryBundle { - /// Bundle file name, resolved at {product}/bundle/{file} on the CDN. + /// Bundle file name, resolved at bundle/{product}/{file} on the CDN (or entry file at changelog/{repo}/{file} for the entry index). public string? File { get; init; } /// Target version or release date declared by the bundle (e.g. 9.3.0). diff --git a/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs b/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs index 56c15e630..d259da7a5 100644 --- a/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs +++ b/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs @@ -100,7 +100,7 @@ public class ChangelogBlock(DirectiveBlockParser parser, ParserContext context) /// /// Product to source bundles for from the public CDN (the :cdn: option). When set, the - /// directive fetches {cdnBase}/{product}/registry.json and the bundles it lists + /// directive fetches {cdnBase}/bundle/{product}/registry.json and the bundles it lists /// instead of reading a local folder; any folder argument is ignored. /// public string? CdnProduct { get; private set; } diff --git a/src/infra/docs-lambda-changelog-scrubber/Program.cs b/src/infra/docs-lambda-changelog-scrubber/Program.cs index 9ce65c731..edc0e2ae5 100644 --- a/src/infra/docs-lambda-changelog-scrubber/Program.cs +++ b/src/infra/docs-lambda-changelog-scrubber/Program.cs @@ -181,7 +181,10 @@ async Task CopyPassThrough(IAmazonS3 s3Client, string sourceBucket, string key, async Task ScrubContent(string key, string content, ILambdaContext context) { - var isBundlePath = key.Contains("/bundle/", StringComparison.OrdinalIgnoreCase); + // Artifact-root layout: bundles live under "bundle/{product}/...", entries under + // "changelog/{repo}/...". Match the bundle prefix (not a "/bundle/" substring, which no longer + // appears in the new keys) so bundles are not misclassified as changelog entries. + var isBundlePath = key.StartsWith("bundle/", StringComparison.OrdinalIgnoreCase); if (isBundlePath) return await ScrubBundle(content, context); diff --git a/src/infra/docs-lambda-changelog-scrubber/README.md b/src/infra/docs-lambda-changelog-scrubber/README.md index 42551ee0d..7b38b2bd4 100644 --- a/src/infra/docs-lambda-changelog-scrubber/README.md +++ b/src/infra/docs-lambda-changelog-scrubber/README.md @@ -31,12 +31,12 @@ The `bootstrap` binary should be available under: ## Event handling - **`s3:ObjectCreated:*`** on `.yaml`/`.yml` files: read from private bucket, scrub private references, write to public bucket -- **`s3:ObjectCreated:*`** on `.json` files: only per-product registry manifests (keys matching `RegistryKey.IsRegistry`, i.e. `{product}/registry.json`) are passed through as-is; any other `.json` key is skipped +- **`s3:ObjectCreated:*`** on `.json` files: only registry manifests (keys matching `RegistryKey.IsRegistry`, i.e. `bundle/{product}/registry.json` or `changelog/{repo}/registry.json`) are passed through as-is; any other `.json` key is skipped - **`s3:ObjectRemoved:*`**: delete the same key from the public bucket - Other keys are silently skipped ## Scrubbing logic -- **Bundle files** (`{product}/bundle/*.yaml`): `LinkAllowlistSanitizer.TryApplyBundle` scrubs `prs`/`issues` lists -- **Changelog entries** (`{product}/changelog/*.yaml`): `LinkAllowlistSanitizer.TryApplyChangelogEntry` scrubs `prs`, `issues`, `description`, `impact`, `action` +- **Bundle files** (`bundle/{product}/*.yaml`, detected by the `bundle/` key prefix): `LinkAllowlistSanitizer.TryApplyBundle` scrubs `prs`/`issues` lists +- **Changelog entries** (`changelog/{repo}/*.yaml`): `LinkAllowlistSanitizer.TryApplyChangelogEntry` scrubs `prs`, `issues`, `description`, `impact`, `action` - The allowlist is built once at cold start from the embedded `assembler.yml` via `BuildAllowReposFromAssembler` diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index 329703a43..86c9d0449 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -2,8 +2,6 @@ // 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; using System.Text.RegularExpressions; @@ -15,7 +13,6 @@ 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; @@ -122,7 +119,7 @@ public record BundlePlanResult public string? OutputPath { get; init; } /// - /// Public CDN URL of the (scrubbed) bundle once uploaded: {base}/{product}/bundle/{file}. + /// Public CDN URL of the (scrubbed) bundle once uploaded: {base}/bundle/{product}/{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. /// @@ -153,8 +150,6 @@ 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(); @@ -214,15 +209,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. 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 + // Decide where the individual changelog entries come from. Under Option AD entries are + // repo-scoped (changelog/{repo}/...), so CDN sourcing keys off the resolvable authoring repo + // (bundle.repo / --repo), not the bundle's target products. Fall back to the local folder + // when the user forces it (bundle.use_local_changelogs / --directory), the repo is + // unresolvable, or no CDN base is configured. This stays in lockstep with PlanBundleAsync's // needs_network decision. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; - var cdnProducts = ResolveCdnProducts(input); - var useCdn = ShouldSourceFromCdn(cdnProducts, useLocalChangelogs, explicitDirectory); + var authoringRepo = NormalizeRepo(input.Repo); + var useCdn = ShouldSourceFromCdn(authoringRepo, useLocalChangelogs, explicitDirectory); // Validate input. In CDN mode the local input directory is not read, so its existence // is not required. @@ -270,7 +265,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle ChangelogMatchResult matchResult; if (useCdn) { - var contents = await FetchCdnEntriesAsync(collector, cdnProducts, ctx); + var contents = await FetchCdnEntriesAsync(collector, authoringRepo, ctx); if (contents == null) return false; matchResult = entryMatcher.MatchChangelogContents(collector, contents, filterCriteria, ctx); @@ -649,14 +644,13 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } } - // 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. + // CDN entry sourcing needs network access for the Docker bundle run. Mirror the run-mode gate: + // active when the authoring repo resolves (profile/config bundle.repo), the user has not forced + // local sourcing, and a CDN base is configured. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var explicitDirectory = !string.IsNullOrWhiteSpace(input.Directory); - var cdnProducts = ResolveCdnProducts(input, profileDef); - if (ShouldSourceFromCdn(cdnProducts, useLocalChangelogs, explicitDirectory)) + var authoringRepo = NormalizeRepo(input.Repo ?? profileDef?.Repo ?? config?.Bundle?.Repo); + if (ShouldSourceFromCdn(authoringRepo, useLocalChangelogs, explicitDirectory)) needsNetwork = true; // Resolve output path — mirrors the logic in ProcessProfile + ApplyConfigDefaults. @@ -686,7 +680,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } /// - /// Public CDN URL of the scrubbed bundle ({base}/{product}/bundle/{file}), polled by the + /// Public CDN URL of the scrubbed bundle ({base}/bundle/{product}/{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) @@ -706,7 +700,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; var basePath = baseUri.AbsoluteUri.TrimEnd('/'); - return $"{basePath}/{Uri.EscapeDataString(product)}/bundle/{Uri.EscapeDataString(fileName)}"; + return $"{basePath}/bundle/{Uri.EscapeDataString(product)}/{Uri.EscapeDataString(fileName)}"; } /// @@ -739,21 +733,22 @@ 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) on any fatal fetch failure; an entry not yet public is - /// skipped with a warning. + /// Downloads the changelog entries from the public CDN for the authoring 's + /// pool (changelog/{repo}/...). Returns null (after emitting an error) on any fatal fetch + /// failure; an entry not yet public is skipped with a warning by the fetcher. The bundle command's + /// own filters (products / prs / issues) then narrow the downloaded set. /// private async Task?> FetchCdnEntriesAsync( IDiagnosticsCollector collector, - IReadOnlyList products, + string? repo, Cancel ctx) { - if (products.Count == 0) + if (string.IsNullOrWhiteSpace(repo)) { collector.EmitError(string.Empty, - "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."); + "Sourcing changelog entries from the CDN requires a resolvable authoring repository. " + + "Set bundle.repo in changelog.yml (or pass --repo), or set bundle.use_local_changelogs: true " + + "in changelog.yml / pass --directory to bundle local changelog files."); return null; } @@ -765,20 +760,13 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; } - var byName = new Dictionary(StringComparer.Ordinal); var fatalFailure = false; - foreach (var product in products) - { - var entries = await _entryFetcher.FetchAsync( - 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; - } + var entries = await _entryFetcher.FetchAsync( + baseUri, + repo, + msg => { fatalFailure = true; collector.EmitError(string.Empty, msg); }, + msg => collector.EmitWarning(string.Empty, msg), + ctx); // 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 @@ -786,108 +774,39 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments if (fatalFailure) return null; - _logger.LogInformation("Sourced {Count} changelog entr(ies) from the CDN for product(s) {Products}", - byName.Count, string.Join(", ", products)); + var byName = new Dictionary(StringComparer.Ordinal); + foreach (var entry in entries) + byName[entry.FileName] = entry.Content; + + _logger.LogInformation("Sourced {Count} changelog entr(ies) from the CDN for repo {Repo}", + byName.Count, repo); return byName.Select(kv => (kv.Key, kv.Value)).ToList(); } /// - /// 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. + /// Reduces a configured repo value to the single path segment used in CDN keys + /// (owner/repo -> repo). Null/empty input is returned unchanged. /// - 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(); - } - - private static void AppendConcreteProducts(List ids, IReadOnlyList? products) + private static string? NormalizeRepo(string? repo) { - if (products == null) - return; - foreach (var product in products) - { - if (!string.IsNullOrWhiteSpace(product.Product) && product.Product != "*") - ids.Add(product.Product); - } - } - - // Profile product patterns are comma-separated groups; each group is whitespace-delimited - // "product target lifecycle", so the product id is the first token of each group. - private static void AppendConcreteProductsFromPattern(List 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); - } + if (string.IsNullOrWhiteSpace(repo)) + return repo; + var slash = repo.LastIndexOf('/'); + return slash >= 0 && slash < repo.Length - 1 ? repo[(slash + 1)..] : repo; } /// - /// 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. + /// Gate for CDN entry sourcing under the artifact-root layout. Entries are repo-scoped + /// (changelog/{repo}/...), so the CDN is used when the authoring repo resolves, the user has + /// not forced local sourcing (bundle.use_local_changelogs or an explicit --directory), + /// and a CDN base is configured. Otherwise entries are read from the local folder. /// - private bool ShouldSourceFromCdn(IReadOnlyList cdnProducts, bool useLocalChangelogs, bool explicitDirectory) + private static bool ShouldSourceFromCdn(string? authoringRepo, bool useLocalChangelogs, bool explicitDirectory) { - if (useLocalChangelogs || explicitDirectory || cdnProducts.Count == 0) + if (useLocalChangelogs || explicitDirectory || string.IsNullOrWhiteSpace(authoringRepo)) 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; + return ChangelogCdn.ResolveBaseUri() is not null; } private bool ValidateInput(IDiagnosticsCollector collector, BundleChangelogsArguments input, bool requireDirectoryExists) diff --git a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs index f3f2f70af..7451f965b 100644 --- a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs +++ b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs @@ -29,6 +29,20 @@ public record ChangelogUploadArguments public required string S3BucketName { get; init; } public string? Config { get; init; } public string? Directory { get; init; } + + /// + /// Authoring repository identifier used to scope changelog-entry keys (changelog/{repo}/{file}). + /// Required for uploads; unused for bundle uploads (which are + /// product-scoped from the bundle YAML). Resolved by the CLI via the precedence + /// --repo > bundle.repo > git remote origin. + /// + public string? Repo { get; init; } + + /// + /// GitHub owner, accepted for parity with other changelog commands. Not part of the S3 key + /// (entry keys are scoped by repo only); retained for diagnostics and future use. + /// + public string? Owner { get; init; } } public partial class ChangelogUploadService( @@ -47,8 +61,10 @@ public partial class ChangelogUploadService( [GeneratedRegex(@"^[a-zA-Z0-9_-]+$")] private static partial Regex ProductNameRegex(); - private static readonly YamlDotNet.Serialization.IDeserializer EntryDeserializer = - ReleaseNotesSerialization.GetEntryDeserializer(); + // Authoring repo identifier used as a single S3 path segment (changelog/{repo}/{file}). + // Same character class as product names; "." / ".." are additionally rejected to prevent traversal. + [GeneratedRegex(@"^[a-zA-Z0-9._-]+$")] + private static partial Regex RepoNameRegex(); public async Task Upload(IDiagnosticsCollector collector, ChangelogUploadArguments args, Cancel ctx) { @@ -73,7 +89,12 @@ public async Task Upload(IDiagnosticsCollector collector, ChangelogUploadA var targets = args.ArtifactType == ArtifactType.Bundle ? DiscoverBundleUploadTargets(collector, directory) - : DiscoverUploadTargets(collector, directory); + : DiscoverUploadTargets(collector, directory, args.Repo); + + // Entry uploads abort (rather than no-op) when the repo cannot be resolved: the keys would be + // unscoped and a silent skip would look like "nothing to upload". + if (collector.Errors > 0) + return false; if (targets.Count == 0) { @@ -132,8 +153,19 @@ private async Task RefreshRegistries( } } - internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector collector, string changelogDir) + internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector collector, string changelogDir, string? repo) { + // Option AD: entries live once, under the authoring repo's pool — independent of which products + // later consume them. The repo must be resolvable (CLI --repo > bundle.repo > git remote); a + // missing/invalid value is fatal because every entry key derives from it. + if (string.IsNullOrWhiteSpace(repo) || !RepoNameRegex().IsMatch(repo) || repo is "." or "..") + { + collector.EmitError(string.Empty, + $"A valid repository identifier is required to upload changelog entries (resolved: \"{repo ?? ""}\"). " + + "Set --repo, bundle.repo in changelog.yml, or run inside a checkout with a github.com origin remote."); + return []; + } + var rootDir = _fileSystem.DirectoryInfo.New(changelogDir); var yamlFiles = _fileSystem.Directory.GetFiles(changelogDir, "*.yaml", SearchOption.TopDirectoryOnly) @@ -151,54 +183,14 @@ internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector continue; } - var products = ReadProductsFromFragment(filePath); - if (products.Count == 0) - { - _logger.LogDebug("No products found in {File}, skipping", filePath); - continue; - } - var fileName = _fileSystem.Path.GetFileName(filePath); - - foreach (var product in products) - { - if (!ProductNameRegex().IsMatch(product)) - { - collector.EmitWarning(filePath, $"Skipping invalid product name \"{product}\" (must match [a-zA-Z0-9_-]+)"); - continue; - } - - var s3Key = $"{product}/changelog/{fileName}"; - targets.Add(new UploadTarget(filePath, s3Key)); - } + var s3Key = $"changelog/{repo}/{fileName}"; + targets.Add(new UploadTarget(filePath, s3Key)); } return targets; } - private List ReadProductsFromFragment(string filePath) - { - try - { - var content = _fileSystem.File.ReadAllText(filePath); - var normalized = ReleaseNotesSerialization.NormalizeYaml(content); - var entry = EntryDeserializer.Deserialize(normalized); - if (entry?.Products == null) - return []; - - return entry.Products - .Select(p => p?.Product) - .Where(p => !string.IsNullOrWhiteSpace(p)) - .Select(p => p!) - .ToList(); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Could not read products from {File}", filePath); - return []; - } - } - internal IReadOnlyList DiscoverBundleUploadTargets(IDiagnosticsCollector collector, string bundleDir) { var rootDir = _fileSystem.DirectoryInfo.New(bundleDir); @@ -235,7 +227,7 @@ internal IReadOnlyList DiscoverBundleUploadTargets(IDiagnosticsCol continue; } - var s3Key = $"{product}/bundle/{fileName}"; + var s3Key = $"bundle/{product}/{fileName}"; targets.Add(new UploadTarget(filePath, s3Key)); } } diff --git a/src/services/Elastic.Changelog/Uploading/Registry.cs b/src/services/Elastic.Changelog/Uploading/Registry.cs index e597ed908..88eac25ca 100644 --- a/src/services/Elastic.Changelog/Uploading/Registry.cs +++ b/src/services/Elastic.Changelog/Uploading/Registry.cs @@ -12,7 +12,8 @@ namespace Elastic.Changelog.Uploading; /// bundle files without an S3 listing call. /// /// -/// Stored at {product}/registry.json in the changelog bundles bucket. +/// Stored at bundle/{product}/registry.json (bundle index) or +/// changelog/{repo}/registry.json (changelog-entry index) in the changelog bundles bucket. /// The scrubber Lambda mirrors it verbatim to the public bucket (pass-through). /// public sealed record Registry @@ -23,7 +24,9 @@ public sealed record Registry public int SchemaVersion { get; init; } = 1; /// - /// Product identifier (matches the first segment of the S3 key). + /// Grouping identifier: the product for a bundle index (bundle/{product}/…) or the + /// authoring repo for a changelog-entry index (changelog/{repo}/…) — i.e. the second + /// path segment of the S3 key. Informational only; consumers locate the manifest by key. /// public required string Product { get; init; } @@ -45,8 +48,9 @@ public sealed record Registry public sealed record RegistryBundle { /// - /// Bundle file name (e.g. 9.3.0.yaml or 2025-11.yaml), - /// resolved at {product}/bundle/{file}. + /// Bundle file name (e.g. 9.3.0.yaml or cloud-2025-11.yaml), + /// resolved at bundle/{product}/{file}. For changelog-entry indexes this is the entry + /// file name resolved at changelog/{repo}/{file}. /// public required string File { get; init; } diff --git a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs index d402c8615..bd80de1d4 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs @@ -20,20 +20,20 @@ namespace Elastic.Changelog.Uploading; /// internal enum RegistryScope { - /// The bundle index at {product}/registry.json, listing scrubbed bundle files. + /// The bundle index at bundle/{product}/registry.json, listing scrubbed bundle files. Bundle, - /// The changelog-entry index at {product}/changelog/registry.json, listing individual entry files. + /// The changelog-entry index at changelog/{repo}/registry.json, listing individual entry files. Changelog } /// -/// Refreshes a per-product registry.json manifest in the private bucket after an upload run. +/// Refreshes a 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). +/// (bundle/{product}/registry.json) or the changelog-entry index +/// (changelog/{repo}/registry.json). Each grouping (product or repo) 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 group cannot clobber each other). /// internal sealed class RegistryBuilder( ILoggerFactory logFactory, @@ -70,10 +70,11 @@ public async Task RefreshAsync( Cancel ctx, RegistryScope scope = RegistryScope.Bundle) { - // Each upload target carries a "{product}/{bundle|changelog}/{file}" S3 key. Group by product - // so we can produce one manifest per affected product. + // Each upload target carries an artifact-root S3 key — "bundle/{product}/{file}" or + // "changelog/{repo}/{file}". Group by the scope's second segment (product for bundles, repo for + // entries) so we produce one manifest per affected group. var byProduct = uploadTargets - .Select(t => (Target: t, Product: ExtractProduct(t.S3Key))) + .Select(t => (Target: t, Product: ExtractGroupKey(t.S3Key, scope))) .Where(x => x.Product is not null) .GroupBy(x => x.Product!, StringComparer.Ordinal); @@ -110,20 +111,29 @@ public async Task RefreshAsync( return new RefreshResult(updated, unchanged, failed); } - /// Extracts the leading product segment from a {product}/bundle/{file} or {product}/changelog/{file} S3 key, or null. - private static string? ExtractProduct(string s3Key) + /// + /// Extracts the grouping segment from an artifact-root S3 key: the product from + /// bundle/{product}/{file} (bundle scope) or the repo from changelog/{repo}/{file} + /// (changelog scope). Returns null when the key does not match the expected scope prefix/shape. + /// + private static string? ExtractGroupKey(string s3Key, RegistryScope scope) { - var firstSlash = s3Key.IndexOf('/'); - if (firstSlash <= 0) + var prefix = scope == RegistryScope.Changelog ? "changelog/" : "bundle/"; + if (!s3Key.StartsWith(prefix, StringComparison.Ordinal)) + return null; + + var rest = s3Key.AsSpan(prefix.Length); + var slash = rest.IndexOf('/'); + if (slash <= 0) return null; - return s3Key.AsSpan(0, firstSlash).ToString(); + return rest[..slash].ToString(); } - /// The S3 key of the per-product manifest for the given . - private static string RegistryKeyFor(string product, RegistryScope scope) => scope switch + /// The S3 key of the manifest for the given and grouping segment. + private static string RegistryKeyFor(string group, RegistryScope scope) => scope switch { - RegistryScope.Changelog => $"{product}/changelog/registry.json", - _ => $"{product}/registry.json" + RegistryScope.Changelog => $"changelog/{group}/registry.json", + _ => $"bundle/{group}/registry.json" }; /// Builds manifest entries for this run's bundles, recording the per- target for the bundle index. diff --git a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs index e25519b49..e352422b7 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs @@ -10,44 +10,50 @@ namespace Elastic.Changelog.Uploading; /// public static class RegistryKey { - private const string BundleSuffix = "/registry.json"; - private const string ChangelogSuffix = "/changelog/registry.json"; + private const string RegistrySuffix = "/registry.json"; + private const string BundlePrefix = "bundle/"; + private const string ChangelogPrefix = "changelog/"; /// - /// 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_-]+). + /// Returns true when is a manifest of either artifact-root form + /// bundle/{product}/registry.json (the bundle index) or + /// changelog/{repo}/registry.json (the changelog-entry index), where the middle + /// segment is a single path segment in the character class [a-zA-Z0-9._-]+. /// /// /// Used by the changelog scrubber Lambda to decide whether to pass an incoming - /// *.json object through to the public bucket. Anything else (e.g. nested - /// under a bundle/ prefix, or a multi-segment product) is rejected, which keeps - /// arbitrary JSON out of the public surface. + /// *.json object through to the public bucket. Anything else (a bare + /// {x}/registry.json, a deeper nesting, or an unknown top-level prefix) is + /// rejected, which keeps arbitrary JSON out of the public surface. /// public static bool IsRegistry(string key) { if (string.IsNullOrEmpty(key)) return false; - if (key.EndsWith(ChangelogSuffix, StringComparison.Ordinal)) - return IsValidProduct(key.AsSpan(0, key.Length - ChangelogSuffix.Length)); + return IsScopedRegistry(key, BundlePrefix) || IsScopedRegistry(key, ChangelogPrefix); + } - if (key.EndsWith(BundleSuffix, StringComparison.Ordinal)) - return IsValidProduct(key.AsSpan(0, key.Length - BundleSuffix.Length)); + private static bool IsScopedRegistry(string key, string prefix) + { + if (!key.StartsWith(prefix, StringComparison.Ordinal) || !key.EndsWith(RegistrySuffix, StringComparison.Ordinal)) + return false; + + var segmentLength = key.Length - prefix.Length - RegistrySuffix.Length; + if (segmentLength <= 0) + return false; - return false; + return IsValidSegment(key.AsSpan(prefix.Length, segmentLength)); } - private static bool IsValidProduct(ReadOnlySpan product) + private static bool IsValidSegment(ReadOnlySpan segment) { - if (product.IsEmpty || product.Contains('/')) + if (segment.IsEmpty || segment.Contains('/') || segment is "." or "..") return false; - foreach (var c in product) + foreach (var c in segment) { - if (!(char.IsAsciiLetterOrDigit(c) || c == '_' || c == '-')) + if (!(char.IsAsciiLetterOrDigit(c) || c == '_' || c == '-' || c == '.')) return false; } return true; diff --git a/src/tooling/docs-builder/Commands/ChangelogCommand.cs b/src/tooling/docs-builder/Commands/ChangelogCommand.cs index 2d699592c..f3ff7bfc6 100644 --- a/src/tooling/docs-builder/Commands/ChangelogCommand.cs +++ b/src/tooling/docs-builder/Commands/ChangelogCommand.cs @@ -1572,12 +1572,22 @@ private string ApplyChangelogInitBundleRepoSeed(string content, string? ownerCli } /// Upload changelog entries or bundle artifacts to S3 or Elasticsearch. - /// Uses content-hash–based incremental transfer — only changed files are uploaded. + /// + /// Uses content-hash–based incremental transfer — only changed files are uploaded. + /// + /// Changelog entries are uploaded once under changelog/{repo}/{file}, keyed by the authoring + /// repository (resolved from --repo, then bundle.repo in changelog.yml, then the git + /// remote origin). Bundles are uploaded under bundle/{product}/{file}, product-scoped from + /// the bundle YAML, and do not require a repo. + /// + /// /// Artifact type to upload: 'changelog' (individual entries) or 'bundle' (consolidated bundles). /// Upload destination: 's3' or 'elasticsearch'. /// S3 bucket name (required when target is 's3'). /// Path to changelog.yml configuration file. Defaults to docs/changelog.yml. /// Override changelog directory instead of reading it from config. + /// GitHub repository name used to scope uploaded changelog entry keys (changelog/{repo}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads. + /// GitHub repository owner, accepted for parity with other changelog commands. Falls back to bundle.owner in changelog.yml, then the git remote origin. Not part of the S3 key. [NoOptionsInjection] public async Task Upload( string artifactType, @@ -1585,6 +1595,8 @@ public async Task Upload( string s3BucketName = "", [Existing, ExpandUserProfile, RejectSymbolicLinks, FileExtensions(Extensions = "yml,yaml")] FileInfo? config = null, [ExpandUserProfile, RejectSymbolicLinks] DirectoryInfo? directory = null, + string? repo = null, + string? owner = null, CancellationToken ct = default ) { @@ -1610,6 +1622,10 @@ public async Task Upload( var resolvedDirectory = directory != null ? directory?.FullName : null; var resolvedConfig = config != null ? config?.FullName : null; + // Resolve the authoring repo for entry keys: --repo > bundle.repo (changelog.yml) > git remote + // origin. Reduced to a single path segment (owner/repo -> repo) for the changelog/{repo}/ key. + var (resolvedRepo, resolvedOwner) = await ResolveUploadRepoOwner(repo, owner, resolvedConfig, ctx); + await using var serviceInvoker = new ServiceInvoker(collector); var service = new ChangelogUploadService(logFactory, configurationContext); var args = new ChangelogUploadArguments @@ -1618,7 +1634,9 @@ public async Task Upload( Target = parsedTarget, S3BucketName = s3BucketName, Config = resolvedConfig, - Directory = resolvedDirectory + Directory = resolvedDirectory, + Repo = resolvedRepo, + Owner = resolvedOwner }; serviceInvoker.AddCommand(service, args, static async (s, c, state, ct) => await s.Upload(c, state, ct) @@ -1626,6 +1644,36 @@ static async (s, c, state, ct) => await s.Upload(c, state, ct) return await serviceInvoker.InvokeAsync(ctx); } + /// + /// Resolves the authoring repo and owner for changelog uploads using the precedence + /// --repo/--owner > bundle.repo/bundle.owner in changelog.yml > git + /// remote origin. The repo is reduced to a single path segment (owner/repo -> repo). + /// + private async Task<(string? Repo, string? Owner)> ResolveUploadRepoOwner(string? repoCli, string? ownerCli, string? configPath, CancellationToken ctx) + { + var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem) + .LoadChangelogConfiguration(collector, configPath, ctx); + + string? gitOwner = null; + string? gitRepo = null; + var cwd = Directory.GetCurrentDirectory(); + var repoRoot = Paths.FindGitRoot(_fileSystem.DirectoryInfo.New(cwd))?.FullName ?? cwd; + if (GitRemoteConfigurationReader.TryReadOriginUrl(_fileSystem, repoRoot, out var originUrl)) + _ = GitHubRemoteParser.TryParseGitHubComOwnerRepo(originUrl, out gitOwner, out gitRepo); + + var resolvedRepo = !string.IsNullOrWhiteSpace(repoCli) ? repoCli : (bundleConfig?.Bundle?.Repo ?? gitRepo); + var resolvedOwner = ownerCli ?? bundleConfig?.Bundle?.Owner ?? gitOwner; + + if (!string.IsNullOrWhiteSpace(resolvedRepo)) + { + var slash = resolvedRepo.LastIndexOf('/'); + if (slash >= 0 && slash < resolvedRepo.Length - 1) + resolvedRepo = resolvedRepo[(slash + 1)..]; + } + + return (resolvedRepo, resolvedOwner); + } + /// /// Normalizes a file path by expanding tilde (~) to the user's home directory /// and converting relative paths to absolute paths. diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs index 090053b1a..bdd1f4c1a 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs @@ -50,7 +50,7 @@ public class BundleCdnSourcingTests(ITestOutputHelper output) : ChangelogTestBas private static StubHandler RegistryHandler() => new(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + if (path.EndsWith("/registry.json", StringComparison.Ordinal)) return Json(RegistryJson); if (path.EndsWith("1-alpha.yaml", StringComparison.Ordinal)) return Yaml(EntryAlpha); @@ -69,16 +69,19 @@ private string OutputPath() => FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "bundle.yaml"); [Fact] - public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() + public async Task OptionMode_RepoResolvable_SourcesAllEntriesFromRepoPoolOnCdn() { - DeclareReleaseNotesProducts("elasticsearch"); - var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher()); + // Under the artifact-root layout the CDN entry pool is keyed by the authoring repo, not the + // target product. A resolvable repo (here via --repo) is what enables CDN sourcing. + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher(Output, handler)); var output = OutputPath(); var input = new BundleChangelogsArguments { InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], Output = output, + Repo = "elasticsearch", Resolve = true }; @@ -87,6 +90,9 @@ public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); Collector.Errors.Should().Be(0); + // Entries are sourced from the authoring repo's pool: changelog/{repo}/... + handler.RequestedPaths.Should().Contain("/changelog/elasticsearch/registry.json"); + var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); bundle.Should().Contain("Alpha"); bundle.Should().Contain("Bravo"); @@ -94,11 +100,11 @@ public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() } [Fact] - public async Task OptionMode_NoResolvableProduct_FallsBackToLocal() + public async Task OptionMode_NoResolvableRepo_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. + // With no --repo, no bundle.repo in config, and no git-remote resolution at the service layer, the + // authoring repo is unresolvable. With no --directory and no use_local_changelogs, the bundler + // still falls back to local folder sourcing rather than hitting the CDN. var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); FileSystem.Directory.CreateDirectory(localDir); await FileSystem.File.WriteAllTextAsync( @@ -129,11 +135,10 @@ await FileSystem.File.WriteAllTextAsync( } [Fact] - public async Task OptionMode_UndeclaredProduct_FallsBackToLocal() + public async Task OptionMode_UseLocalChangelogs_ForcesLocalEvenWithResolvableRepo() { - // 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. + // use_local_changelogs is the explicit opt-out: even when the authoring repo resolves (bundle.repo), + // entries are read from the local folder and the CDN is never touched. var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); FileSystem.Directory.CreateDirectory(localDir); await FileSystem.File.WriteAllTextAsync( @@ -143,6 +148,8 @@ await FileSystem.File.WriteAllTextAsync( $""" bundle: directory: {localDir} + repo: elasticsearch + use_local_changelogs: true """; var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); @@ -163,7 +170,7 @@ await FileSystem.File.WriteAllTextAsync( 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"); + handler.RequestedPaths.Should().BeEmpty("use_local_changelogs must not reach the CDN"); var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); bundle.Should().Contain("name: 1-local.yaml"); @@ -172,7 +179,6 @@ await FileSystem.File.WriteAllTextAsync( [Fact] public async Task RegistryFailure_FailsBundle() { - DeclareReleaseNotesProducts("elasticsearch"); var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)), sleep: (_, _) => Task.CompletedTask); var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); @@ -181,6 +187,7 @@ public async Task RegistryFailure_FailsBundle() { InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], Output = OutputPath(), + Repo = "elasticsearch", Resolve = true }; @@ -198,20 +205,20 @@ public async Task EntryMissingAfterRetries_FailsBundle() var handler = new StubHandler(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + if (path.EndsWith("/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: (_, _) => Task.CompletedTask); - DeclareReleaseNotesProducts("elasticsearch"); var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); var input = new BundleChangelogsArguments { InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], Output = OutputPath(), + Repo = "elasticsearch", Resolve = true }; @@ -224,9 +231,8 @@ public async Task EntryMissingAfterRetries_FailsBundle() [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. - DeclareReleaseNotesProducts("elasticsearch"); + // A github_release profile resolves the authoring repo from the profile (to scope the CDN entry + // pool) 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); diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs index ff3b56625..75070688a 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs @@ -53,16 +53,16 @@ public async Task Plan_ReleaseVersion_ReturnsNeedsNetwork() } [Fact] - public async Task Plan_ProfileMode_ResolvesOutputPath() + public async Task Plan_ProfileMode_RepoResolvable_ReturnsNeedsNetwork() { - // A profile whose product is declared under release_notes sources entries from the CDN, so the plan + // A profile whose authoring repo resolves (bundle.repo) 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 = """ bundle: output_directory: docs/releases + repo: elasticsearch profiles: my-profile: products: "elasticsearch {version} {lifecycle}" @@ -83,16 +83,16 @@ public async Task Plan_ProfileMode_ResolvesOutputPath() 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"); + // The bundle-PR action polls this URL for the scrubbed copy: {base}/bundle/{product}/{file}. + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/bundle/elasticsearch/elasticsearch-9.2.0.yaml"); } [Fact] - public async Task Plan_ProfileMode_UndeclaredProduct_ReturnsNoNetwork() + public async Task Plan_ProfileMode_NoRepo_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. + // Repo gate at plan time: with no bundle.repo (and no --repo), the authoring repo is unresolvable, + // so the bundle run sources locally and 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 = """ @@ -116,7 +116,7 @@ public async Task Plan_ProfileMode_UndeclaredProduct_ReturnsNoNetwork() result.Should().NotBeNull(); result.NeedsNetwork.Should().BeFalse(); - result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/elasticsearch/bundle/elasticsearch-9.2.0.yaml"); + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/bundle/elasticsearch/elasticsearch-9.2.0.yaml"); } [Fact] @@ -146,7 +146,7 @@ public async Task Plan_ProfileMode_OutputProductsScopeCdnUrl() 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"); + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/bundle/cloud-serverless/serverless-2026-03.yaml"); } [Fact] diff --git a/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs new file mode 100644 index 000000000..ef4e73b11 --- /dev/null +++ b/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs @@ -0,0 +1,196 @@ +// 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.Documentation.Configuration; +using Elastic.Documentation.Configuration.ReleaseNotes; +using Elastic.Documentation.Diagnostics; + +namespace Elastic.Changelog.Tests.Changelogs; + +/// +/// End-to-end fixture modelled on the non-trivial elastic/cloud changelog configuration +/// (multi-product, a monthly profile, bundle.repo with repo != product, link_allow_repos, +/// resolve: true, release_dates: false, and rules.bundle.exclude_types), but using +/// anonymized repo and product names only — never the real org/repo/product IDs. +/// +/// It verifies the artifact-root (Option AD) behaviour: entries are sourced once from the authoring +/// repo's pool (changelog/{repo}/...), and the profile bundle that results is sound — resolved, +/// link-scrubbed against the allowlist, with the docs entry excluded and no auto-populated release date. +/// +/// The authoring repo is an anonymized test name (widget) that deliberately differs from the +/// product IDs it publishes for — exactly the repo != product "moniker mismatch" that motivates Option +/// AD. Product IDs are drawn from the test harness's products.yml allowlist. +/// +public class CloudProfileFixtureTests(ITestOutputHelper output) : ChangelogTestBase(output) +{ + private const string AuthoringRepo = "widget"; + + // A feature entry whose only PR points at an allowlisted public repo (kept on scrub) plus a PR to a + // non-allowlisted repo (scrubbed away). + // language=yaml + private const string FeatureEntry = """ + title: Faster hosted search + type: feature + products: + - product: cloud-hosted + target: 2026-05-15 + lifecycle: ga + prs: + - https://github.com/elastic/elasticsearch/pull/100 + - https://github.com/elastic/widget-internal/pull/7 + """; + + // A docs entry that matches the product/target filter but must be dropped by rules.bundle.exclude_types. + // language=yaml + private const string DocsEntry = """ + title: Tidy up the hosted docs + type: docs + products: + - product: cloud-hosted + target: 2026-05-20 + lifecycle: ga + prs: + - https://github.com/elastic/elasticsearch/pull/200 + """; + + // A feature for a different product; excluded by the profile's cloud-hosted product filter. + // language=yaml + private const string OtherProductEntry = """ + title: Serverless-only change + type: feature + products: + - product: cloud-serverless + target: 2026-05-10 + lifecycle: ga + prs: + - https://github.com/elastic/kibana/pull/300 + """; + + // language=json + private const string RegistryJson = + """{ "schema_version": 1, "product": "widget", "bundles": [ { "file": "1-feature.yaml" }, { "file": "2-docs.yaml" }, { "file": "3-other.yaml" } ] }"""; + + private StubHandler RepoPoolHandler() => new(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/registry.json", StringComparison.Ordinal)) + return Json(RegistryJson); + if (path.EndsWith("1-feature.yaml", StringComparison.Ordinal)) + return Yaml(FeatureEntry); + if (path.EndsWith("2-docs.yaml", StringComparison.Ordinal)) + return Yaml(DocsEntry); + if (path.EndsWith("3-other.yaml", StringComparison.Ordinal)) + return Yaml(OtherProductEntry); + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + + private CdnChangelogEntryFetcher Fetcher(StubHandler handler) => + new(new TestLoggerFactory(Output), handler, sleep: (_, _) => Task.CompletedTask); + + [Fact] + public async Task MonthlyProfile_SourcesFromRepoPool_ProducesSoundBundle() + { + var outputDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString()); + FileSystem.Directory.CreateDirectory(outputDir); + + // language=yaml + var configContent = + """ + products: + available: + - cloud-hosted + - cloud-serverless + pivot: + types: + feature: ">feature" + bug-fix: ">bug" + breaking-change: ">breaking" + docs: ">docs" + rules: + bundle: + exclude_types: "docs" + bundle: + output_directory: PLACEHOLDER + resolve: true + repo: widget + owner: elastic + release_dates: false + link_allow_repos: + - elastic/elasticsearch + - elastic/kibana + profiles: + wh-monthly: + products: "cloud-hosted {version}-* *" + output: "widget-{version}.yaml" + output_products: "cloud-hosted {version}" + """.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 handler = RepoPoolHandler(); + var service = new ChangelogBundlingService(LoggerFactory, ConfigurationContext, FileSystem, null, Fetcher(handler)); + + var input = new BundleChangelogsArguments + { + Profile = "wh-monthly", + ProfileArgument = "2026-05", + 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); + + // Entries are sourced once from the authoring repo's pool, not from any product-scoped path. + handler.RequestedPaths.Should().Contain($"/changelog/{AuthoringRepo}/registry.json"); + handler.RequestedPaths.Should().NotContain(p => p.Contains("/cloud-hosted/changelog/", StringComparison.Ordinal)); + + var outputFiles = FileSystem.Directory.GetFiles(outputDir, "*.yaml"); + outputFiles.Should().ContainSingle("the monthly profile writes a single bundle file"); + FileSystem.Path.GetFileName(outputFiles[0]).Should().Be("widget-2026-05.yaml"); + + var bundle = await FileSystem.File.ReadAllTextAsync(outputFiles[0], TestContext.Current.CancellationToken); + + // Sound bundle: the matching feature is present and resolved; the docs entry and the other-product + // entry are excluded. + bundle.Should().Contain("Faster hosted search"); + bundle.Should().NotContain("Tidy up the hosted docs", "rules.bundle.exclude_types drops docs entries"); + bundle.Should().NotContain("Serverless-only change", "the cloud-hosted product filter excludes other products"); + + // Link allowlist: the allowlisted public PR is kept verbatim; the non-allowlisted repo reference is + // rewritten to a "# PRIVATE:" sentinel rather than left as a live link. + bundle.Should().Contain("- https://github.com/elastic/elasticsearch/pull/100"); + bundle.Should().Contain("# PRIVATE: https://github.com/elastic/widget-internal/pull/7", + "non-allowlisted PR links must be scrubbed to a PRIVATE sentinel in bundle output"); + + // release_dates: false → no auto-populated release date. + bundle.Should().NotContain("release_date"); + } + + 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/LinkAllowlistSanitizerTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs index d93d7e5e0..9b8db13e4 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs @@ -534,7 +534,7 @@ public void TryApplyChangelogEntry_NullFields_PreservesNulls() public void TryApplyChangelogEntry_BarePrNumberWithoutDefaultRepo_KeptWithWarning() { // The scrubber Lambda calls TryApplyChangelogEntry with defaultRepo=null because per-entry - // YAMLs uploaded under {product}/changelog/*.yaml carry no embedded repo context. A bare + // YAMLs uploaded under changelog/{repo}/*.yaml carry no embedded repo context. A bare // numeric PR ref ("155500") must be tolerated rather than failing the whole entry — the // reference carries no repo identity so it cannot leak a private link, and downstream // rendering supplies the owner/repo from runtime context. diff --git a/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs b/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs index 8887ef24e..f737ea405 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs @@ -48,7 +48,7 @@ private string AddChangelog(string fileName, string yaml) } [Fact] - public void DiscoverUploadTargets_SingleProduct_MapsToCorrectS3Key() + public void DiscoverUploadTargets_SingleEntry_MapsToRepoScopedKey() { // language=yaml var path = AddChangelog("entry.yaml", """ @@ -61,17 +61,19 @@ public void DiscoverUploadTargets_SingleProduct_MapsToCorrectS3Key() - "100" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); targets.Should().HaveCount(1); targets[0].LocalPath.Should().Be(path); - targets[0].S3Key.Should().Be("elasticsearch/changelog/entry.yaml"); + targets[0].S3Key.Should().Be("changelog/elasticsearch/entry.yaml"); _collector.Errors.Should().Be(0); } [Fact] - public void DiscoverUploadTargets_MultipleProducts_CreatesTargetPerProduct() + public void DiscoverUploadTargets_EntryWithMultipleProducts_StillSingleRepoKey() { + // Option AD: entries are stored once per authoring repo, regardless of how many products they + // list (or will later be consumed by). No per-product fan-out. // language=yaml AddChangelog("fix.yaml", """ title: Cross-product fix @@ -85,35 +87,17 @@ public void DiscoverUploadTargets_MultipleProducts_CreatesTargetPerProduct() - "200" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "kibana"); - targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/changelog/fix.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/changelog/fix.yaml"); + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be("changelog/kibana/fix.yaml"); } [Fact] - public void DiscoverUploadTargets_InvalidProductName_SkipsWithWarning() - { - // language=yaml - AddChangelog("bad.yaml", """ - title: Bad product - type: feature - products: - - product: "../traversal" - prs: - - "300" - """); - - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); - - targets.Should().BeEmpty(); - _collector.Warnings.Should().BeGreaterThan(0); - } - - [Fact] - public void DiscoverUploadTargets_NoProducts_ReturnsEmpty() + public void DiscoverUploadTargets_EntryWithNoProducts_StillUploaded() { + // Author foreknowledge of consuming products is no longer required: an entry with no products is + // still uploaded under the repo pool. // language=yaml AddChangelog("noproducts.yaml", """ title: No products @@ -122,45 +106,62 @@ public void DiscoverUploadTargets_NoProducts_ReturnsEmpty() - "400" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); - targets.Should().BeEmpty(); + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be("changelog/elasticsearch/noproducts.yaml"); _collector.Errors.Should().Be(0); } [Fact] - public void DiscoverUploadTargets_EmptyDirectory_ReturnsEmpty() + public void DiscoverUploadTargets_MissingRepo_EmitsErrorAndReturnsEmpty() { - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, repo: null); targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); } - [Fact] - public void DiscoverUploadTargets_MixedValidAndInvalidProducts_FiltersCorrectly() + [Theory] + [InlineData("bad repo")] + [InlineData("..")] + [InlineData(".")] + [InlineData("owner/repo")] + public void DiscoverUploadTargets_InvalidRepo_EmitsError(string repo) { // language=yaml - AddChangelog("mixed.yaml", """ - title: Mixed products + AddChangelog("entry.yaml", """ + title: New feature type: feature products: - product: elasticsearch - - product: "bad product with spaces" - - product: kibana - prs: - - "500" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, repo); - targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/changelog/mixed.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/changelog/mixed.yaml"); - _collector.Warnings.Should().BeGreaterThan(0); + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); } [Fact] - public void DiscoverUploadTargets_MultipleFiles_DiscoversBoth() + public void DiscoverUploadTargets_EmptyDirectory_ReturnsEmpty() + { + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().Be(0); + } + + [Fact] + public void DiscoverUploadTargets_MultipleFiles_DiscoversAllUnderRepo() { // language=yaml AddChangelog("first.yaml", """ @@ -181,32 +182,34 @@ public void DiscoverUploadTargets_MultipleFiles_DiscoversBoth() - "2" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/changelog/first.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/changelog/second.yaml"); + targets.Should().Contain(t => t.S3Key == "changelog/elasticsearch/first.yaml"); + targets.Should().Contain(t => t.S3Key == "changelog/elasticsearch/second.yaml"); } - [Fact] - public void DiscoverUploadTargets_ProductWithHyphensAndUnderscores_Accepted() + [Theory] + [InlineData("elasticsearch-net")] + [InlineData("cloud_hosted")] + [InlineData("apm-agent-dotnet")] + [InlineData("docs.elastic")] + public void DiscoverUploadTargets_RepoWithHyphensDotsUnderscores_Accepted(string repo) { // language=yaml - AddChangelog("hyphen.yaml", """ + AddChangelog("entry.yaml", """ title: Hyphenated type: feature products: - - product: elastic-agent - - product: cloud_hosted + - product: elasticsearch prs: - "600" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, repo); - targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elastic-agent/changelog/hyphen.yaml"); - targets.Should().Contain(t => t.S3Key == "cloud_hosted/changelog/hyphen.yaml"); + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be($"changelog/{repo}/entry.yaml"); _collector.Errors.Should().Be(0); _collector.Warnings.Should().Be(0); } @@ -236,7 +239,8 @@ public async Task Upload_WithValidChangelogs_UploadsToS3() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Repo = "elasticsearch" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -245,20 +249,53 @@ public async Task Upload_WithValidChangelogs_UploadsToS3() _collector.Errors.Should().Be(0); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/changelog/entry.yaml" && r.BucketName == "test-bucket"), + A.That.Matches(r => r.Key == "changelog/elasticsearch/entry.yaml" && r.BucketName == "test-bucket"), A._ )).MustHaveHappenedOnceExactly(); } [Fact] - public async Task Upload_EmptyDirectory_ReturnsTrue() + public async Task Upload_ChangelogWithoutRepo_FailsWithoutS3Calls() { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + target: 9.2.0 + prs: + - "100" + """); + var args = new ChangelogUploadArguments { ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", Directory = _changelogDir + // Repo intentionally unset. + }; + var ct = TestContext.Current.CancellationToken; + var result = await _service.Upload(_collector, args, ct); + + result.Should().BeFalse(); + _collector.Errors.Should().BeGreaterThan(0); + + A.CallTo(() => _s3Client.PutObjectAsync(A._, A._)) + .MustNotHaveHappened(); + } + + [Fact] + public async Task Upload_EmptyDirectory_ReturnsTrue() + { + var args = new ChangelogUploadArguments + { + ArtifactType = ArtifactType.Changelog, + Target = UploadTargetKind.S3, + S3BucketName = "test-bucket", + Directory = _changelogDir, + Repo = "elasticsearch" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -294,7 +331,8 @@ public async Task Upload_WithFailedUpload_ReturnsFalseAndEmitsError() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Repo = "elasticsearch" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -375,13 +413,13 @@ public async Task Upload_BundleArtifactType_UploadsToS3() _collector.Errors.Should().Be(0); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/bundle/elasticsearch-9.2.0.yaml" && r.BucketName == "test-bucket"), + A.That.Matches(r => r.Key == "bundle/elasticsearch/elasticsearch-9.2.0.yaml" && r.BucketName == "test-bucket"), A._ )).MustHaveHappenedOnceExactly(); } [Fact] - public void DiscoverBundleUploadTargets_MapsToCorrectS3Key() + public void DiscoverBundleUploadTargets_MapsToArtifactRootKey() { var bundleDir = _mockFileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "releases"); _mockFileSystem.Directory.CreateDirectory(bundleDir); @@ -405,7 +443,7 @@ public void DiscoverBundleUploadTargets_MapsToCorrectS3Key() var targets = _service.DiscoverBundleUploadTargets(_collector, bundleDir); targets.Should().HaveCount(1); - targets[0].S3Key.Should().Be("elasticsearch/bundle/elasticsearch-9.2.0.yaml"); + targets[0].S3Key.Should().Be("bundle/elasticsearch/elasticsearch-9.2.0.yaml"); _collector.Errors.Should().Be(0); } @@ -436,8 +474,8 @@ public void DiscoverBundleUploadTargets_MultipleProducts_CreatesTargetPerProduct var targets = _service.DiscoverBundleUploadTargets(_collector, bundleDir); targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/bundle/stack-9.2.0.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/bundle/stack-9.2.0.yaml"); + targets.Should().Contain(t => t.S3Key == "bundle/elasticsearch/stack-9.2.0.yaml"); + targets.Should().Contain(t => t.S3Key == "bundle/kibana/stack-9.2.0.yaml"); } [Fact] @@ -492,18 +530,18 @@ public async Task Upload_BundleArtifactType_UploadsRegistryAlongsideBundle() _collector.Errors.Should().Be(0); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/bundle/9.3.0.yaml"), + A.That.Matches(r => r.Key == "bundle/elasticsearch/9.3.0.yaml"), A._ )).MustHaveHappenedOnceExactly(); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/registry.json"), + A.That.Matches(r => r.Key == "bundle/elasticsearch/registry.json"), A._ )).MustHaveHappenedOnceExactly(); } [Fact] - public async Task Upload_ChangelogArtifactType_DoesNotUploadRegistry() + public async Task Upload_ChangelogArtifactType_RefreshesRepoScopedRegistry() { // language=yaml AddChangelog("entry.yaml", """ @@ -518,6 +556,8 @@ public async Task Upload_ChangelogArtifactType_DoesNotUploadRegistry() A.CallTo(() => _s3Client.GetObjectMetadataAsync(A._, A._)) .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); + A.CallTo(() => _s3Client.GetObjectAsync(A._, A._)) + .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); A.CallTo(() => _s3Client.PutObjectAsync(A._, A._)) .Returns(new PutObjectResponse()); @@ -526,15 +566,22 @@ public async Task Upload_ChangelogArtifactType_DoesNotUploadRegistry() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Repo = "elasticsearch" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); result.Should().BeTrue(); + // Changelog uploads refresh the repo-scoped entry index, not a bundle index. + A.CallTo(() => _s3Client.PutObjectAsync( + A.That.Matches(r => r.Key == "changelog/elasticsearch/registry.json"), + A._ + )).MustHaveHappenedOnceExactly(); + A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key.EndsWith("/registry.json", StringComparison.Ordinal)), + A.That.Matches(r => r.Key.StartsWith("bundle/", StringComparison.Ordinal)), A._ )).MustNotHaveHappened(); } diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs index d773c17d9..01fe9de97 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs @@ -84,7 +84,7 @@ private void StubExistingManifestNotFound() => private void StubExistingManifest(string product, Registry manifest, string etag = "\"existing-etag\"") => A.CallTo(() => _s3Client.GetObjectAsync( - A.That.Matches(r => r.Key == $"{product}/registry.json"), + A.That.Matches(r => r.Key == $"bundle/{product}/registry.json"), A._)) .ReturnsLazily(() => MakeManifestResponse(manifest, etag)); @@ -105,7 +105,7 @@ private static Registry Deserialize(string? json) => public async Task Refresh_NoExistingManifest_CreatesManifestWithIfNoneMatch() { var path = AddBundle("9.3.0.yaml", "elasticsearch", "9.3.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.3.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.3.0.yaml") }; StubExistingManifestNotFound(); var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -113,7 +113,7 @@ public async Task Refresh_NoExistingManifest_CreatesManifestWithIfNoneMatch() result.Updated.Should().Be(1); _puts.Should().ContainSingle(); var put = _puts[0]; - put.Key.Should().Be("elasticsearch/registry.json"); + put.Key.Should().Be("bundle/elasticsearch/registry.json"); put.IfNoneMatch.Should().Be("*"); put.IfMatch.Should().BeNull(); @@ -145,8 +145,8 @@ public async Task Refresh_ExistingManifest_MergesByFileNameAndUsesIfMatch() var ten = AddBundle("9.4.0.yaml", "elasticsearch", "9.4.0"); var targets = new List { - new(newer, "elasticsearch/bundle/9.3.0.yaml"), - new(ten, "elasticsearch/bundle/9.4.0.yaml") + new(newer, "bundle/elasticsearch/9.3.0.yaml"), + new(ten, "bundle/elasticsearch/9.4.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -177,8 +177,8 @@ public async Task Refresh_SortsManifestByVersionNotLexicographically() var v99 = AddBundle("9.9.0.yaml", "elasticsearch", "9.9.0"); var targets = new List { - new(v99, "elasticsearch/bundle/9.9.0.yaml"), - new(v910, "elasticsearch/bundle/9.10.0.yaml") + new(v99, "bundle/elasticsearch/9.9.0.yaml"), + new(v910, "bundle/elasticsearch/9.10.0.yaml") }; _ = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -194,8 +194,8 @@ public async Task Refresh_MultipleProducts_WritesOneManifestPerProduct() var kb = AddBundle("kb-9.3.0.yaml", "kibana", "9.3.0"); var targets = new List { - new(es, "elasticsearch/bundle/9.3.0.yaml"), - new(kb, "kibana/bundle/kb-9.3.0.yaml") + new(es, "bundle/elasticsearch/9.3.0.yaml"), + new(kb, "bundle/kibana/kb-9.3.0.yaml") }; StubExistingManifestNotFound(); @@ -203,8 +203,8 @@ public async Task Refresh_MultipleProducts_WritesOneManifestPerProduct() result.Updated.Should().Be(2); _puts.Should().HaveCount(2); - _puts.Should().Contain(p => p.Key == "elasticsearch/registry.json"); - _puts.Should().Contain(p => p.Key == "kibana/registry.json"); + _puts.Should().Contain(p => p.Key == "bundle/elasticsearch/registry.json"); + _puts.Should().Contain(p => p.Key == "bundle/kibana/registry.json"); } [Fact] @@ -232,17 +232,17 @@ public async Task Refresh_MultiProductBundle_RecordsTargetPerProduct() """)); var targets = new List { - new(path, "elasticsearch/bundle/multi.yaml"), - new(path, "kibana/bundle/multi.yaml") + new(path, "bundle/elasticsearch/multi.yaml"), + new(path, "bundle/kibana/multi.yaml") }; StubExistingManifestNotFound(); _ = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); - var es = Deserialize(_puts.Single(p => p.Key == "elasticsearch/registry.json").ContentBody); + var es = Deserialize(_puts.Single(p => p.Key == "bundle/elasticsearch/registry.json").ContentBody); es.Bundles[0].Target.Should().Be("9.3.0"); - var kb = Deserialize(_puts.Single(p => p.Key == "kibana/registry.json").ContentBody); + var kb = Deserialize(_puts.Single(p => p.Key == "bundle/kibana/registry.json").ContentBody); kb.Bundles[0].Target.Should().Be("9.4.0"); } @@ -257,7 +257,7 @@ public async Task Refresh_ExistingManifestUnreadable_OverwritesUsingLiveETag() }); var path = AddBundle("9.3.0.yaml", "elasticsearch", "9.3.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.3.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.3.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -277,7 +277,7 @@ public async Task Refresh_BundleWithoutTarget_RecordsEntryWithoutTarget() _mockFileSystem.AddFile(path, new MockFileData(""" entries: [] """)); - var targets = new List { new(path, "elasticsearch/bundle/no-target.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/no-target.yaml") }; StubExistingManifestNotFound(); var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -305,7 +305,7 @@ public async Task Refresh_UnchangedManifest_SkipsWrite() }; StubExistingManifest("elasticsearch", existing, "\"manifest-v1\""); - var targets = new List { new(path, "elasticsearch/bundle/9.3.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.3.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); result.Unchanged.Should().Be(1); @@ -344,7 +344,7 @@ public async Task Refresh_ConcurrentWrite_RetriesAfterPreconditionFailed() .Once(); var path = AddBundle("9.4.0.yaml", "elasticsearch", "9.4.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.4.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.4.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -379,7 +379,7 @@ public async Task Refresh_PersistentConcurrentWrite_EmitsWarningAndReportsFailur .Throws(new AmazonS3Exception("Precondition Failed") { StatusCode = HttpStatusCode.PreconditionFailed }); var path = AddBundle("9.4.0.yaml", "elasticsearch", "9.4.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.4.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.4.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -408,7 +408,7 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() - product: elasticsearch target: 9.3.0 """)); - var targets = new List { new(path, "elasticsearch/changelog/1-feature.yaml") }; + var targets = new List { new(path, "changelog/elasticsearch/1-feature.yaml") }; A.CallTo(() => _s3Client.GetObjectAsync(A._, A._)) .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); @@ -416,7 +416,7 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() result.Updated.Should().Be(1); _puts.Should().ContainSingle(); - _puts[0].Key.Should().Be("elasticsearch/changelog/registry.json"); + _puts[0].Key.Should().Be("changelog/elasticsearch/registry.json"); var manifest = Deserialize(_puts[0].ContentBody); manifest.Product.Should().Be("elasticsearch"); diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs index bd0091a22..2654f9b14 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs @@ -10,33 +10,45 @@ 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) => + // Bundle index (artifact-root): bundle/{product}/registry.json + [InlineData("bundle/elasticsearch/registry.json")] + [InlineData("bundle/kibana/registry.json")] + [InlineData("bundle/elastic-agent/registry.json")] + [InlineData("bundle/cloud_hosted/registry.json")] + [InlineData("bundle/cloud-serverless/registry.json")] + [InlineData("bundle/a/registry.json")] + // Changelog-entry index (artifact-root): changelog/{repo}/registry.json + [InlineData("changelog/elasticsearch/registry.json")] + [InlineData("changelog/kibana/registry.json")] + [InlineData("changelog/cloud/registry.json")] + // Repo segments may contain dots (e.g. apm-agent-dotnet, elasticsearch-net). + [InlineData("changelog/apm.agent/registry.json")] + public void IsRegistry_ValidArtifactRootKeys_ReturnsTrue(string key) => RegistryKey.IsRegistry(key).Should().BeTrue(); [Theory] [InlineData("")] [InlineData("registry.json")] [InlineData("/registry.json")] + // Old product-first layout is no longer a valid manifest key. + [InlineData("elasticsearch/registry.json")] + [InlineData("elasticsearch/changelog/registry.json")] + // Missing/empty middle segment. + [InlineData("bundle/registry.json")] + [InlineData("changelog/registry.json")] + [InlineData("bundle//registry.json")] + // Unknown top-level prefix. + [InlineData("entries/elasticsearch/registry.json")] [InlineData("elasticsearch/bundle/registry.json")] - [InlineData("elasticsearch/registry.yaml")] - [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")] + // Wrong extension. + [InlineData("bundle/elasticsearch/registry.yaml")] + [InlineData("changelog/elasticsearch/registry.yaml")] + // Deeper nesting / traversal in the middle segment. + [InlineData("bundle/elastic/search/registry.json")] + [InlineData("changelog/elastic/search/registry.json")] + [InlineData("bundle/../registry.json")] + [InlineData("changelog/../registry.json")] + [InlineData("bundle/elastic search/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 index 9839e7a35..109ae13fc 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs @@ -37,7 +37,7 @@ private static (List Errors, List Warnings, Action EmitE public async Task FetchAsync_HappyPath_ReturnsAllEntriesFromRegistry() { var handler = new StubHandler(req => - req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) + req.RequestUri!.AbsolutePath.EndsWith("/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(); @@ -49,7 +49,9 @@ public async Task FetchAsync_HappyPath_ReturnsAllEntriesFromRegistry() 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)); + // Artifact-root layout: entries and their registry live under changelog/{repo}/... + handler.RequestedPaths.Should().Contain("/changelog/elasticsearch/registry.json"); + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/changelog/elasticsearch/1-a.yaml", StringComparison.Ordinal)); } [Fact] @@ -72,7 +74,7 @@ public async Task FetchAsync_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty( // (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)) + if (req.RequestUri!.AbsolutePath.EndsWith("/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) @@ -97,7 +99,7 @@ public async Task FetchAsync_EntryRecoversAfterRetry_ReturnsEntry() var handler = new StubHandler(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + if (path.EndsWith("/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 @@ -134,7 +136,7 @@ public async Task FetchAsync_SchemaVersionTooNew_EmitsError() public async Task FetchAsync_UnsafeFileName_EmitsWarningAndSkips() { var handler = new StubHandler(req => - req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) + req.RequestUri!.AbsolutePath.EndsWith("/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(); diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs index 8de403a05..1a071f96b 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs @@ -53,6 +53,10 @@ public async Task FetchAsync_HappyPath_ReturnsBundlesFromRegistry() bundles.Should().ContainSingle(); bundles[0].Version.Should().Be("9.3.0"); bundles[0].Entries.Should().ContainSingle().Which.Title.Should().Be("Sample enhancement"); + + // Artifact-root layout: bundles and their registry live under bundle/{product}/... + handler.RequestedPaths.Should().Contain("/bundle/elasticsearch/registry.json"); + handler.RequestedPaths.Should().Contain("/bundle/elasticsearch/9.3.0.yaml"); } [Fact] From 0fc045fbdc265bc1614f104ef32b6ee852182b4f Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Thu, 25 Jun 2026 09:31:33 -0300 Subject: [PATCH 2/6] changelog: address CodeRabbit review on S3 restructure - RegistryKey: scope dot characters to changelog/{repo} segments; bundle product segments stay [a-zA-Z0-9_-]+ (matches producer validation), so the scrubber no longer passes through bundle/foo.bar/registry.json - CLI upload: anchor the git-remote repo fallback to the --config/--directory source rather than the process cwd, so an out-of-tree config resolves the right origin - docs: fix the producer-details bullet to describe repo-scoped entry registries - condense private-method XML doc summaries to a single line Skipped CodeRabbit's NormalizeRepo '+' suggestion: the config loader already rejects '+' in bundle.repo, so merged values never reach key construction. Refs elastic/docs-eng-team#413 Co-authored-by: Cursor --- docs/development/changelog-bundle-registry.md | 4 +-- .../Bundling/ChangelogBundlingService.cs | 24 +++-------------- .../Uploading/RegistryBuilder.cs | 6 +---- .../Uploading/RegistryKey.cs | 14 ++++++---- .../docs-builder/Commands/ChangelogCommand.cs | 27 ++++++++++++------- .../Uploading/RegistryKeyTests.cs | 3 +++ 6 files changed, 37 insertions(+), 41 deletions(-) diff --git a/docs/development/changelog-bundle-registry.md b/docs/development/changelog-bundle-registry.md index a8e3befce..0bcfc82b0 100644 --- a/docs/development/changelog-bundle-registry.md +++ b/docs/development/changelog-bundle-registry.md @@ -110,8 +110,8 @@ skipped for `--artifact-type changelog`). `RegistryBuilder`: - Groups the run's upload targets by the second key segment — product for bundles (`bundle/{product}/{file}`), repo for entries (`changelog/{repo}/{file}`). -- For each product, derives one `registry` entry per bundle (file name, that product's - target, locally-computed S3 ETag). +- For each group, derives one `registry` entry per file (file name, locally-computed S3 ETag, + and — for bundle indexes only — that product's target; entry indexes record no target). - Reads the existing manifest from S3, merges by file name (re-uploads replace their entry; others are preserved), and writes the merged manifest back. diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index 86c9d0449..953ac35f7 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -679,10 +679,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments }; } - /// - /// Public CDN URL of the scrubbed bundle ({base}/bundle/{product}/{file}), polled by the - /// bundle-PR action. Null when the product, output file name, or CDN base cannot be resolved. - /// + /// Public CDN URL of the scrubbed bundle ({base}/bundle/{product}/{file}); null when product, output file name, or CDN base cannot be resolved. private string? ResolveCdnBundleUrl(BundleProfile? profileDef, BundleChangelogsArguments input, string? outputPath) { if (string.IsNullOrWhiteSpace(outputPath)) @@ -732,12 +729,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; } - /// - /// Downloads the changelog entries from the public CDN for the authoring 's - /// pool (changelog/{repo}/...). Returns null (after emitting an error) on any fatal fetch - /// failure; an entry not yet public is skipped with a warning by the fetcher. The bundle command's - /// own filters (products / prs / issues) then narrow the downloaded set. - /// + /// Downloads the authoring 's changelog entries from the CDN (changelog/{repo}/...); returns null after emitting an error on any fatal fetch failure. private async Task?> FetchCdnEntriesAsync( IDiagnosticsCollector collector, string? repo, @@ -784,10 +776,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return byName.Select(kv => (kv.Key, kv.Value)).ToList(); } - /// - /// Reduces a configured repo value to the single path segment used in CDN keys - /// (owner/repo -> repo). Null/empty input is returned unchanged. - /// + /// Reduces a configured repo value to the single CDN-key path segment (owner/repo -> repo); null/empty unchanged. private static string? NormalizeRepo(string? repo) { if (string.IsNullOrWhiteSpace(repo)) @@ -796,12 +785,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return slash >= 0 && slash < repo.Length - 1 ? repo[(slash + 1)..] : repo; } - /// - /// Gate for CDN entry sourcing under the artifact-root layout. Entries are repo-scoped - /// (changelog/{repo}/...), so the CDN is used when the authoring repo resolves, the user has - /// not forced local sourcing (bundle.use_local_changelogs or an explicit --directory), - /// and a CDN base is configured. Otherwise entries are read from the local folder. - /// + /// Gate for repo-scoped CDN entry sourcing: true when the authoring repo resolves, local sourcing is not forced (bundle.use_local_changelogs/--directory), and a CDN base is configured. private static bool ShouldSourceFromCdn(string? authoringRepo, bool useLocalChangelogs, bool explicitDirectory) { if (useLocalChangelogs || explicitDirectory || string.IsNullOrWhiteSpace(authoringRepo)) diff --git a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs index bd80de1d4..21dd9305c 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs @@ -111,11 +111,7 @@ public async Task RefreshAsync( return new RefreshResult(updated, unchanged, failed); } - /// - /// Extracts the grouping segment from an artifact-root S3 key: the product from - /// bundle/{product}/{file} (bundle scope) or the repo from changelog/{repo}/{file} - /// (changelog scope). Returns null when the key does not match the expected scope prefix/shape. - /// + /// Extracts the grouping segment (product for bundle/{product}/…, repo for changelog/{repo}/…) from an artifact-root S3 key, or null. private static string? ExtractGroupKey(string s3Key, RegistryScope scope) { var prefix = scope == RegistryScope.Changelog ? "changelog/" : "bundle/"; diff --git a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs index e352422b7..337ae3967 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs @@ -17,8 +17,9 @@ public static class RegistryKey /// /// Returns true when is a manifest of either artifact-root form /// bundle/{product}/registry.json (the bundle index) or - /// changelog/{repo}/registry.json (the changelog-entry index), where the middle - /// segment is a single path segment in the character class [a-zA-Z0-9._-]+. + /// changelog/{repo}/registry.json (the changelog-entry index). The bundle product + /// segment matches the producer's product class ([a-zA-Z0-9_-]+); the changelog repo + /// segment additionally allows . (GitHub repo names can contain dots). /// /// /// Used by the changelog scrubber Lambda to decide whether to pass an incoming @@ -43,17 +44,20 @@ private static bool IsScopedRegistry(string key, string prefix) if (segmentLength <= 0) return false; - return IsValidSegment(key.AsSpan(prefix.Length, segmentLength)); + // Bundle products never contain dots (producer class is [a-zA-Z0-9_-]+); only changelog repo + // segments may, so dots are accepted under changelog/ but not bundle/. + var allowDots = string.Equals(prefix, ChangelogPrefix, StringComparison.Ordinal); + return IsValidSegment(key.AsSpan(prefix.Length, segmentLength), allowDots); } - private static bool IsValidSegment(ReadOnlySpan segment) + private static bool IsValidSegment(ReadOnlySpan segment, bool allowDots) { if (segment.IsEmpty || segment.Contains('/') || segment is "." or "..") return false; foreach (var c in segment) { - if (!(char.IsAsciiLetterOrDigit(c) || c == '_' || c == '-' || c == '.')) + if (!(char.IsAsciiLetterOrDigit(c) || c == '_' || c == '-' || (allowDots && c == '.'))) return false; } return true; diff --git a/src/tooling/docs-builder/Commands/ChangelogCommand.cs b/src/tooling/docs-builder/Commands/ChangelogCommand.cs index f3ff7bfc6..de913cd79 100644 --- a/src/tooling/docs-builder/Commands/ChangelogCommand.cs +++ b/src/tooling/docs-builder/Commands/ChangelogCommand.cs @@ -1624,7 +1624,7 @@ public async Task Upload( // Resolve the authoring repo for entry keys: --repo > bundle.repo (changelog.yml) > git remote // origin. Reduced to a single path segment (owner/repo -> repo) for the changelog/{repo}/ key. - var (resolvedRepo, resolvedOwner) = await ResolveUploadRepoOwner(repo, owner, resolvedConfig, ctx); + var (resolvedRepo, resolvedOwner) = await ResolveUploadRepoOwner(repo, owner, resolvedConfig, resolvedDirectory, ctx); await using var serviceInvoker = new ServiceInvoker(collector); var service = new ChangelogUploadService(logFactory, configurationContext); @@ -1644,20 +1644,29 @@ static async (s, c, state, ct) => await s.Upload(c, state, ct) return await serviceInvoker.InvokeAsync(ctx); } - /// - /// Resolves the authoring repo and owner for changelog uploads using the precedence - /// --repo/--owner > bundle.repo/bundle.owner in changelog.yml > git - /// remote origin. The repo is reduced to a single path segment (owner/repo -> repo). - /// - private async Task<(string? Repo, string? Owner)> ResolveUploadRepoOwner(string? repoCli, string? ownerCli, string? configPath, CancellationToken ctx) + /// Resolves the authoring repo/owner for uploads (--repo > bundle.repo > git remote), reducing the repo to a single path segment. + private async Task<(string? Repo, string? Owner)> ResolveUploadRepoOwner(string? repoCli, string? ownerCli, string? configPath, string? uploadDirectory, CancellationToken ctx) { var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem) .LoadChangelogConfiguration(collector, configPath, ctx); + // Anchor the git-remote fallback to the upload source (config file or changelog directory), not + // the process cwd, so an out-of-tree --config/--directory resolves the right origin. Both values + // are absolute (FileInfo/DirectoryInfo FullName) when present. + string? anchor = null; + if (!string.IsNullOrWhiteSpace(configPath)) + { + var configDir = _fileSystem.Path.GetDirectoryName(configPath); + if (!string.IsNullOrWhiteSpace(configDir) && _fileSystem.Directory.Exists(configDir)) + anchor = configDir; + } + if (anchor is null && !string.IsNullOrWhiteSpace(uploadDirectory) && _fileSystem.Directory.Exists(uploadDirectory)) + anchor = uploadDirectory; + anchor ??= Directory.GetCurrentDirectory(); + string? gitOwner = null; string? gitRepo = null; - var cwd = Directory.GetCurrentDirectory(); - var repoRoot = Paths.FindGitRoot(_fileSystem.DirectoryInfo.New(cwd))?.FullName ?? cwd; + var repoRoot = Paths.FindGitRoot(_fileSystem.DirectoryInfo.New(anchor))?.FullName ?? anchor; if (GitRemoteConfigurationReader.TryReadOriginUrl(_fileSystem, repoRoot, out var originUrl)) _ = GitHubRemoteParser.TryParseGitHubComOwnerRepo(originUrl, out gitOwner, out gitRepo); diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs index 2654f9b14..9ca9eac5a 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs @@ -40,6 +40,9 @@ public void IsRegistry_ValidArtifactRootKeys_ReturnsTrue(string key) => // Unknown top-level prefix. [InlineData("entries/elasticsearch/registry.json")] [InlineData("elasticsearch/bundle/registry.json")] + // Dots are allowed only for changelog repo segments, never for bundle product segments + // (producers validate products as [a-zA-Z0-9_-]+). + [InlineData("bundle/foo.bar/registry.json")] // Wrong extension. [InlineData("bundle/elasticsearch/registry.yaml")] [InlineData("changelog/elasticsearch/registry.yaml")] From 820e382e9c9cb5ddcbc8e5aa151dc2e7ede4bf85 Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Thu, 25 Jun 2026 10:57:03 -0300 Subject: [PATCH 3/6] changelog: name boolean arguments at call sites (style review) Per the docs-builder boolean-parameter rule, pass useLocalChangelogs/ explicitDirectory (ShouldSourceFromCdn) and allowDots (IsValidSegment) as named arguments at their call sites. Refs elastic/docs-eng-team#413 Co-authored-by: Cursor --- .../Elastic.Changelog/Bundling/ChangelogBundlingService.cs | 4 ++-- src/services/Elastic.Changelog/Uploading/RegistryKey.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index 953ac35f7..80604cfeb 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -217,7 +217,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // needs_network decision. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var authoringRepo = NormalizeRepo(input.Repo); - var useCdn = ShouldSourceFromCdn(authoringRepo, useLocalChangelogs, explicitDirectory); + var useCdn = ShouldSourceFromCdn(authoringRepo, useLocalChangelogs: useLocalChangelogs, explicitDirectory: explicitDirectory); // Validate input. In CDN mode the local input directory is not read, so its existence // is not required. @@ -650,7 +650,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var explicitDirectory = !string.IsNullOrWhiteSpace(input.Directory); var authoringRepo = NormalizeRepo(input.Repo ?? profileDef?.Repo ?? config?.Bundle?.Repo); - if (ShouldSourceFromCdn(authoringRepo, useLocalChangelogs, explicitDirectory)) + if (ShouldSourceFromCdn(authoringRepo, useLocalChangelogs: useLocalChangelogs, explicitDirectory: explicitDirectory)) needsNetwork = true; // Resolve output path — mirrors the logic in ProcessProfile + ApplyConfigDefaults. diff --git a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs index 337ae3967..b0b2567fc 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs @@ -47,7 +47,7 @@ private static bool IsScopedRegistry(string key, string prefix) // Bundle products never contain dots (producer class is [a-zA-Z0-9_-]+); only changelog repo // segments may, so dots are accepted under changelog/ but not bundle/. var allowDots = string.Equals(prefix, ChangelogPrefix, StringComparison.Ordinal); - return IsValidSegment(key.AsSpan(prefix.Length, segmentLength), allowDots); + return IsValidSegment(key.AsSpan(prefix.Length, segmentLength), allowDots: allowDots); } private static bool IsValidSegment(ReadOnlySpan segment, bool allowDots) From 741da09d74a6922da467f7f7ddd3894153069ff2 Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Tue, 30 Jun 2026 11:14:05 -0300 Subject: [PATCH 4/6] changelog: nest entry keys under {org}/{repo}/{branch} Extends the artifact-root layout so changelog entries are stored at changelog/{org}/{repo}/{branch}/{file} (and registry.json), keyed by the authoring org, repo, and branch. This supports changelogs from GitHub orgs other than elastic (e.g. acquired companies) and from branches other than main. The branch is stored verbatim, so a branch's '/' (feature/foo) and '.' (8.x) become real key segments. Bundles are unchanged. - Producer: ChangelogUploadService validates org/repo/branch and builds the nested key; Owner/Branch added to the upload arguments. - Registry: RegistryBuilder groups by the {org}/{repo}/{branch} prefix; RegistryKey.IsRegistry accepts >=3 changelog segments (bundle stays one). - Consumer: CdnChangelogEntryFetcher takes org/repo/branch (slash-aware); ChangelogBundlingService resolves org (bundle.owner > elastic) and branch (bundle.branch > main) when sourcing from the CDN. - Config/CLI: add bundle.branch and --branch to upload/bundle; producer branch falls back to the current checkout's branch. - cli-schema.json regenerated; tests and docs updated. --- config/changelog.example.yml | 3 + docs/cli-schema.json | 20 +- .../Changelog/BundleConfiguration.cs | 13 ++ .../Changelog/ChangelogConfigurationLoader.cs | 2 + .../Changelog/ChangelogConfigurationYaml.cs | 10 + .../ReleaseNotes/CdnChangelogEntryFetcher.cs | 66 ++++--- .../ReleaseNotes/ChangelogRegistry.cs | 4 +- .../docs-lambda-changelog-scrubber/Program.cs | 4 +- .../docs-lambda-changelog-scrubber/README.md | 4 +- .../Bundling/ChangelogBundlingService.cs | 50 +++-- .../Uploading/ChangelogUploadService.cs | 72 +++++-- .../Elastic.Changelog/Uploading/Registry.cs | 10 +- .../Uploading/RegistryBuilder.cs | 47 ++++- .../Uploading/RegistryKey.cs | 47 +++-- .../docs-builder/Commands/ChangelogCommand.cs | 50 +++-- .../Changelogs/BundleCdnSourcingTests.cs | 29 ++- .../Changelogs/CloudProfileFixtureTests.cs | 7 +- .../Changelogs/LinkAllowlistSanitizerTests.cs | 2 +- .../Uploading/ChangelogUploadServiceTests.cs | 175 +++++++++++++++--- .../Uploading/RegistryBuilderTests.cs | 35 +++- .../Uploading/RegistryKeyTests.cs | 40 ++-- .../CdnChangelogEntryFetcherTests.cs | 38 +++- 22 files changed, 568 insertions(+), 160 deletions(-) diff --git a/config/changelog.example.yml b/config/changelog.example.yml index e4327e719..8c62b4e4c 100644 --- a/config/changelog.example.yml +++ b/config/changelog.example.yml @@ -236,6 +236,9 @@ bundle: # repo: elasticsearch # Optional: default GitHub owner applied to all profiles that do not specify their own. # owner: elastic + # Optional: branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/...) is sourced from when + # bundling entries from the CDN. Defaults to "main" when unset. Can be overridden per profile. + # branch: main # Optional: control auto-population of release-date for all profiles by default. # When true (default), auto-populate release dates. Profiles can override this setting. # release_dates: true diff --git a/docs/cli-schema.json b/docs/cli-schema.json index 5aa8dc608..85a7f271b 100644 --- a/docs/cli-schema.json +++ b/docs/cli-schema.json @@ -2779,6 +2779,13 @@ "required": false, "summary": "GitHub repository owner, which is used when PRs or issues are specified as numbers or when using --release-version. Falls back to bundle.owner in changelog.yml when not specified. If that value is also absent, \u0022elastic\u0022 is used." }, + { + "role": "flag", + "name": "branch", + "type": "string", + "required": false, + "summary": "Branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/...) entries are sourced from. Falls back to bundle.branch in changelog.yml, then \u0022main\u0022." + }, { "role": "flag", "name": "plan", @@ -3898,7 +3905,7 @@ ], "name": "upload", "summary": "Upload changelog entries or bundle artifacts to S3 or Elasticsearch.", - "notes": "Uses content-hash\u2013based incremental transfer \u2014 only changed files are uploaded.\n\n\nChangelog entries are uploaded once under changelog/{repo}/{file}, keyed by the authoring\nrepository (resolved from --repo, then bundle.repo in changelog.yml, then the git\nremote origin). Bundles are uploaded under bundle/{product}/{file}, product-scoped from\nthe bundle YAML, and do not require a repo.", + "notes": "Uses content-hash\u2013based incremental transfer \u2014 only changed files are uploaded.\n\n\nChangelog entries are uploaded once under changelog/{org}/{repo}/{branch}/{file}, keyed by the\nauthoring owner (--owner \u003E bundle.owner \u003E git remote), repository (--repo\n\u003E bundle.repo \u003E git remote), and branch (--branch \u003E the current checkout\u0027s\nbranch). The branch is stored verbatim, so a branch\u0027s / become real key separators. Bundles\nare uploaded under bundle/{product}/{file}, product-scoped from the bundle YAML, and do not\nrequire an owner/repo/branch.", "usage": "docs-builder changelog upload --artifact-type \u003Cstring\u003E --target \u003Cstring\u003E [options]", "examples": [], "parameters": [ @@ -3962,14 +3969,21 @@ "name": "repo", "type": "string", "required": false, - "summary": "GitHub repository name used to scope uploaded changelog entry keys (changelog/{repo}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads." + "summary": "GitHub repository name, the second segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads." }, { "role": "flag", "name": "owner", "type": "string", "required": false, - "summary": "GitHub repository owner, accepted for parity with other changelog commands. Falls back to bundle.owner in changelog.yml, then the git remote origin. Not part of the S3 key." + "summary": "GitHub owner (org), the first segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.owner in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads." + }, + { + "role": "flag", + "name": "branch", + "type": "string", + "required": false, + "summary": "Branch, the third segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...), stored verbatim. Falls back to the current checkout\u0027s branch. Required for changelog uploads; ignored for bundle uploads." }, { "role": "flag", diff --git a/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs b/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs index f838ffb8c..6e0d121c4 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs @@ -52,6 +52,13 @@ public record BundleConfiguration /// public string? Owner { get; init; } + /// + /// Branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/…) entries are sourced from + /// when bundling from the CDN. Applied to all profiles that do not specify their own. Defaults to + /// main when unset. + /// + public string? Branch { get; init; } + /// /// When set (including an empty list), PR/issue references whose resolved owner/repo is not listed /// are rewritten to # PRIVATE: sentinels at bundle time. When absent, no link filtering is applied. @@ -118,6 +125,12 @@ public record BundleProfile /// public string? Owner { get; init; } + /// + /// Branch whose CDN changelog pool entries this profile sources from. Overrides + /// when set. + /// + public string? Branch { get; init; } + /// /// Feature IDs to mark as hidden in the bundle output. /// When the bundle is rendered, entries with matching feature-id values will be commented out. diff --git a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs index 2106bf52d..732505501 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs @@ -519,6 +519,7 @@ private static PivotConfiguration ConvertPivot(PivotConfigurationYaml yamlPivot) Description = kvp.Value.Description, Repo = kvp.Value.Repo, Owner = kvp.Value.Owner, + Branch = kvp.Value.Branch, HideFeatures = kvp.Value.HideFeatures?.Values, ReleaseDates = kvp.Value.ReleaseDates, Source = kvp.Value.Source @@ -534,6 +535,7 @@ private static PivotConfiguration ConvertPivot(PivotConfigurationYaml yamlPivot) Description = yaml.Description, Repo = yaml.Repo, Owner = yaml.Owner, + Branch = yaml.Branch, ReleaseDates = yaml.ReleaseDates, LinkAllowRepos = linkAllowRepos, Profiles = profiles diff --git a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs index 41f628acc..0e80be7af 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs @@ -301,6 +301,11 @@ internal sealed record BundleConfigurationYaml /// public string? Owner { get; set; } + /// + /// Branch whose CDN changelog pool entries are sourced from when bundling. Defaults to main. + /// + public string? Branch { get; set; } + /// /// When true, auto-populate release date in bundle output. Defaults to true when omitted. /// @@ -356,6 +361,11 @@ internal sealed record BundleProfileYaml /// public string? Owner { get; set; } + /// + /// Branch whose CDN changelog pool entries this profile sources from. Overrides bundle.branch. + /// + public string? Branch { get; set; } + /// /// Feature IDs to mark as hidden in the bundle output (string or list). /// diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs index f4e48f68f..757002196 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs @@ -14,11 +14,11 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; public readonly record struct CdnChangelogEntry(string FileName, string Content); /// -/// Fetches the individual (scrubbed) changelog entries for a single authoring repo from the public -/// CDN, for the changelog bundle command when sourcing entries from S3 rather than a local -/// folder. It reads {base}/changelog/{repo}/registry.json to enumerate entries and downloads -/// each {base}/changelog/{repo}/{file} as raw YAML; the bundle command then applies its usual -/// filter (products / prs / issues) to the downloaded set. +/// Fetches the individual (scrubbed) changelog entries for a single authoring org/repo/branch pool from +/// the public CDN, for the changelog bundle command when sourcing entries from S3 rather than a +/// local folder. It reads {base}/changelog/{org}/{repo}/{branch}/registry.json to enumerate entries +/// and downloads each {base}/changelog/{org}/{repo}/{branch}/{file} as raw YAML; the bundle command +/// then applies its usual filter (products / prs / issues) to the downloaded set. /// /// /// @@ -92,19 +92,23 @@ public CdnChangelogEntryFetcher( } /// - /// Downloads the changelog entries for the authoring 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. + /// Downloads the changelog entries for the authoring // + /// pool 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 async Task> FetchAsync( Uri baseUri, + string org, string repo, + string branch, Action emitError, Action emitWarning, Cancel ctx) { - var registryUri = Combine(baseUri, "changelog", repo, "registry.json"); + var poolSegments = PoolSegments(org, repo, branch).ToArray(); + var poolLabel = $"{org}/{repo}/{branch}"; + var registryUri = CombineSegments(baseUri, [.. poolSegments, "registry.json"]); ChangelogRegistry? registry; try @@ -113,20 +117,20 @@ public async Task> FetchAsync( } catch (Exception ex) when (ex is not OperationCanceledException) { - emitError($"Could not fetch changelog entry registry for repo '{repo}' from {registryUri}: {ex.Message}"); + emitError($"Could not fetch changelog entry registry for '{poolLabel}' from {registryUri}: {ex.Message}"); return []; } if (registry is null) { - emitError($"Changelog entry registry for repo '{repo}' at {registryUri} was empty or unparseable."); + emitError($"Changelog entry registry for '{poolLabel}' at {registryUri} was empty or unparseable."); return []; } if (registry.SchemaVersion > SupportedSchemaVersion) { emitError( - $"Changelog entry registry for repo '{repo}' uses schema version {registry.SchemaVersion}, but this build only understands version {SupportedSchemaVersion}. Update docs-builder."); + $"Changelog entry registry for '{poolLabel}' uses schema version {registry.SchemaVersion}, but this build only understands version {SupportedSchemaVersion}. Update docs-builder."); return []; } @@ -138,12 +142,12 @@ public async Task> FetchAsync( var fileName = entry.File; if (string.IsNullOrWhiteSpace(fileName) || !IsSafeFileName(fileName)) { - emitWarning($"Changelog entry registry for '{repo}' lists an invalid file name '{fileName}'; skipping."); + emitWarning($"Changelog entry registry for '{poolLabel}' lists an invalid file name '{fileName}'; skipping."); continue; } - var entryUri = Combine(baseUri, "changelog", repo, fileName); - var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, repo, ctx).ConfigureAwait(false); + var entryUri = CombineSegments(baseUri, [.. poolSegments, fileName]); + var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, poolLabel, ctx).ConfigureAwait(false); if (fetched) { entries.Add(new CdnChangelogEntry(fileName, content)); @@ -154,12 +158,12 @@ public async Task> FetchAsync( // 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 repo '{repo}' is listed in the registry but could not be fetched from {entryUri} after {_maxAttempts} attempt(s): {lastError}. " + + $"Changelog entry '{fileName}' for '{poolLabel}' 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 repo {Repo} from {BaseUri}", entries.Count, repo, baseUri); + _logger.LogInformation("Fetched {Count} changelog entry(ies) for {Pool} from {BaseUri}", entries.Count, poolLabel, baseUri); return entries; } @@ -168,7 +172,7 @@ public async Task> FetchAsync( /// 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 async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string repo, Cancel ctx) + private async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string poolLabel, Cancel ctx) { string? lastError = null; @@ -179,7 +183,7 @@ public async Task> FetchAsync( { var content = await FetchTextAsync(uri, attempt, ctx).ConfigureAwait(false); if (attempt > 1) - _logger.LogInformation("Fetched changelog entry '{File}' for {Repo} on attempt {Attempt}/{Max}", fileName, repo, attempt, _maxAttempts); + _logger.LogInformation("Fetched changelog entry '{File}' for {Pool} on attempt {Attempt}/{Max}", fileName, poolLabel, attempt, _maxAttempts); return (true, content, null); } catch (Exception ex) when (ex is not OperationCanceledException) @@ -190,8 +194,8 @@ public async Task> FetchAsync( var delay = RetryDelay(attempt); _logger.LogDebug( - "Changelog entry '{File}' for {Repo} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", - fileName, repo, attempt, _maxAttempts, ex.Message, delay); + "Changelog entry '{File}' for {Pool} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", + fileName, poolLabel, attempt, _maxAttempts, ex.Message, delay); await _sleep(delay, ctx).ConfigureAwait(false); } } @@ -241,16 +245,30 @@ private static Uri WithCacheBuster(Uri uri) return new Uri($"{uri.AbsoluteUri}{separator}_={DateTimeOffset.UtcNow.Ticks:x}"); } - private static Uri Combine(Uri baseUri, params string[] segments) + private static Uri CombineSegments(Uri baseUri, IReadOnlyList segments) { var basePath = baseUri.AbsoluteUri.TrimEnd('/'); var suffix = string.Join('/', segments.Select(Uri.EscapeDataString)); return new Uri($"{basePath}/{suffix}"); } + /// + /// Expands the pool into individually-escapable path segments — changelog, org, repo, then each + /// /-delimited part of the branch — so a branch's slashes stay real CDN key separators rather + /// than being percent-encoded into a single segment. + /// + private static IEnumerable PoolSegments(string org, string repo, string branch) + { + yield return "changelog"; + yield return org; + yield return repo; + foreach (var part in branch.Split('/')) + yield return part; + } + /// /// 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 changelog/{repo}/{file}). + /// must be a single path segment (the producer always writes changelog/{org}/{repo}/{branch}/{file}). /// private static bool IsSafeFileName(string fileName) => !fileName.Contains('/', StringComparison.Ordinal) diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs index 3ded1cd8d..7943918c6 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs @@ -8,7 +8,7 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; /// /// Consumer-side view of the bundle/{product}/registry.json manifest (or the -/// changelog/{repo}/registry.json entry index) published alongside scrubbed changelog content. +/// changelog/{org}/{repo}/{branch}/registry.json entry index) published alongside scrubbed changelog content. /// Mirrors the producer's shape (see the changelog upload service) but is intentionally lenient: only /// the fields the changelog directive needs to enumerate bundles are declared, and nothing is /// required so a partially-written or future-versioned manifest still deserializes. @@ -28,7 +28,7 @@ public sealed record ChangelogRegistry /// One entry in . public sealed record ChangelogRegistryBundle { - /// Bundle file name, resolved at bundle/{product}/{file} on the CDN (or entry file at changelog/{repo}/{file} for the entry index). + /// Bundle file name, resolved at bundle/{product}/{file} on the CDN (or entry file at changelog/{org}/{repo}/{branch}/{file} for the entry index). public string? File { get; init; } /// Target version or release date declared by the bundle (e.g. 9.3.0). diff --git a/src/infra/docs-lambda-changelog-scrubber/Program.cs b/src/infra/docs-lambda-changelog-scrubber/Program.cs index edc0e2ae5..8cd1a4ea6 100644 --- a/src/infra/docs-lambda-changelog-scrubber/Program.cs +++ b/src/infra/docs-lambda-changelog-scrubber/Program.cs @@ -182,8 +182,8 @@ async Task CopyPassThrough(IAmazonS3 s3Client, string sourceBucket, string key, async Task ScrubContent(string key, string content, ILambdaContext context) { // Artifact-root layout: bundles live under "bundle/{product}/...", entries under - // "changelog/{repo}/...". Match the bundle prefix (not a "/bundle/" substring, which no longer - // appears in the new keys) so bundles are not misclassified as changelog entries. + // "changelog/{org}/{repo}/{branch}/...". Match the bundle prefix (not a "/bundle/" substring, which no + // longer appears in the new keys) so bundles are not misclassified as changelog entries. var isBundlePath = key.StartsWith("bundle/", StringComparison.OrdinalIgnoreCase); if (isBundlePath) diff --git a/src/infra/docs-lambda-changelog-scrubber/README.md b/src/infra/docs-lambda-changelog-scrubber/README.md index 7b38b2bd4..6797772a8 100644 --- a/src/infra/docs-lambda-changelog-scrubber/README.md +++ b/src/infra/docs-lambda-changelog-scrubber/README.md @@ -31,12 +31,12 @@ The `bootstrap` binary should be available under: ## Event handling - **`s3:ObjectCreated:*`** on `.yaml`/`.yml` files: read from private bucket, scrub private references, write to public bucket -- **`s3:ObjectCreated:*`** on `.json` files: only registry manifests (keys matching `RegistryKey.IsRegistry`, i.e. `bundle/{product}/registry.json` or `changelog/{repo}/registry.json`) are passed through as-is; any other `.json` key is skipped +- **`s3:ObjectCreated:*`** on `.json` files: only registry manifests (keys matching `RegistryKey.IsRegistry`, i.e. `bundle/{product}/registry.json` or `changelog/{org}/{repo}/{branch}/registry.json`) are passed through as-is; any other `.json` key is skipped - **`s3:ObjectRemoved:*`**: delete the same key from the public bucket - Other keys are silently skipped ## Scrubbing logic - **Bundle files** (`bundle/{product}/*.yaml`, detected by the `bundle/` key prefix): `LinkAllowlistSanitizer.TryApplyBundle` scrubs `prs`/`issues` lists -- **Changelog entries** (`changelog/{repo}/*.yaml`): `LinkAllowlistSanitizer.TryApplyChangelogEntry` scrubs `prs`, `issues`, `description`, `impact`, `action` +- **Changelog entries** (`changelog/{org}/{repo}/{branch}/*.yaml`): `LinkAllowlistSanitizer.TryApplyChangelogEntry` scrubs `prs`, `issues`, `description`, `impact`, `action` - The allowlist is built once at cold start from the embedded `assembler.yml` via `BuildAllowReposFromAssembler` diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index 80604cfeb..96156401d 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -45,6 +45,12 @@ public record BundleChangelogsArguments public string? Owner { get; init; } public string? Repo { get; init; } + /// + /// Branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/…) entries are sourced from. + /// null = use config bundle.branch, then the default branch (main). + /// + public string? Branch { get; init; } + /// /// Profile name to use (from bundle.profiles in config) /// @@ -145,6 +151,10 @@ public partial class ChangelogBundlingService( ? new ChangelogConfigurationLoader(logFactory, configurationContext, fileSystem ?? FileSystemFactory.RealRead) : null; + // Defaults applied when sourcing CDN entries and the org/branch are not otherwise resolvable. + private const string DefaultOwner = "elastic"; + private const string DefaultBranch = "main"; + /// /// UTF-8 encoding without BOM for writing YAML files. /// @@ -209,14 +219,16 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // Apply config defaults if available input = ApplyConfigDefaults(input, config); - // Decide where the individual changelog entries come from. Under Option AD entries are - // repo-scoped (changelog/{repo}/...), so CDN sourcing keys off the resolvable authoring repo - // (bundle.repo / --repo), not the bundle's target products. Fall back to the local folder - // when the user forces it (bundle.use_local_changelogs / --directory), the repo is - // unresolvable, or no CDN base is configured. This stays in lockstep with PlanBundleAsync's - // needs_network decision. + // Decide where the individual changelog entries come from. Under Option AD entries are scoped to + // an org/repo/branch pool (changelog/{org}/{repo}/{branch}/...), so CDN sourcing keys off the + // resolvable authoring repo (bundle.repo / --repo), with org and branch defaulting when unset — + // not the bundle's target products. Fall back to the local folder when the user forces it + // (bundle.use_local_changelogs / --directory), the repo is unresolvable, or no CDN base is + // configured. This stays in lockstep with PlanBundleAsync's needs_network decision. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var authoringRepo = NormalizeRepo(input.Repo); + var authoringOwner = string.IsNullOrWhiteSpace(input.Owner) ? DefaultOwner : input.Owner; + var authoringBranch = string.IsNullOrWhiteSpace(input.Branch) ? DefaultBranch : input.Branch; var useCdn = ShouldSourceFromCdn(authoringRepo, useLocalChangelogs: useLocalChangelogs, explicitDirectory: explicitDirectory); // Validate input. In CDN mode the local input directory is not read, so its existence @@ -265,7 +277,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle ChangelogMatchResult matchResult; if (useCdn) { - var contents = await FetchCdnEntriesAsync(collector, authoringRepo, ctx); + var contents = await FetchCdnEntriesAsync(collector, authoringOwner, authoringRepo, authoringBranch, ctx); if (contents == null) return false; matchResult = entryMatcher.MatchChangelogContents(collector, contents, filterCriteria, ctx); @@ -466,6 +478,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle IReadOnlyList? outputProducts = null; string? repo = null; string? owner = null; + string? branch = null; string[]? mergedHideFeatures = null; string? profileDescription = null; var profileSuppressReleaseDate = false; @@ -506,9 +519,10 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle outputProducts = parsedOutputProducts; } - // Profile-level repo/owner takes precedence; fall back to bundle-level defaults + // Profile-level repo/owner/branch takes precedence; fall back to bundle-level defaults repo = profile.Repo ?? config.Bundle.Repo; owner = profile.Owner ?? config.Bundle.Owner; + branch = profile.Branch ?? config.Bundle.Branch; mergedHideFeatures = profile.HideFeatures?.Count > 0 ? [.. profile.HideFeatures] : null; profileSuppressReleaseDate = !(profile.ReleaseDates ?? config.Bundle.ReleaseDates ?? true); @@ -554,6 +568,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle OutputProducts = outputProducts, Repo = repo, Owner = owner, + Branch = branch, HideFeatures = mergedHideFeatures, Description = profileDescription, SuppressReleaseDate = profileSuppressReleaseDate @@ -576,9 +591,10 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments // Apply resolve: CLI takes precedence over config. Only use config when CLI did not specify. var resolve = input.Resolve ?? config.Bundle.Resolve; - // Apply repo/owner: CLI takes precedence; fall back to bundle-level config defaults. + // Apply repo/owner/branch: CLI takes precedence; fall back to bundle-level config defaults. var repo = input.Repo ?? config.Bundle.Repo; var owner = input.Owner ?? config.Bundle.Owner; + var branch = input.Branch ?? config.Bundle.Branch; // Apply description: CLI takes precedence; fall back to bundle-level config default var description = input.Description ?? config.Bundle.Description; @@ -596,6 +612,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments Resolve = resolve, Repo = repo, Owner = owner, + Branch = branch, Description = description, SuppressReleaseDate = suppressReleaseDate, LinkAllowRepos = config.Bundle.LinkAllowRepos @@ -729,10 +746,12 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; } - /// Downloads the authoring 's changelog entries from the CDN (changelog/{repo}/...); returns null after emitting an error on any fatal fetch failure. + /// Downloads the authoring // pool's changelog entries from the CDN (changelog/{org}/{repo}/{branch}/...); returns null after emitting an error on any fatal fetch failure. private async Task?> FetchCdnEntriesAsync( IDiagnosticsCollector collector, + string? org, string? repo, + string? branch, Cancel ctx) { if (string.IsNullOrWhiteSpace(repo)) @@ -744,6 +763,11 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; } + // org/branch always resolve to a default at the call site, but guard anyway so the fetcher never + // receives a blank pool segment. + var resolvedOrg = string.IsNullOrWhiteSpace(org) ? DefaultOwner : org; + var resolvedBranch = string.IsNullOrWhiteSpace(branch) ? DefaultBranch : branch; + var baseUri = ChangelogCdn.ResolveBaseUri(); if (baseUri is null) { @@ -755,7 +779,9 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments var fatalFailure = false; var entries = await _entryFetcher.FetchAsync( baseUri, + resolvedOrg, repo, + resolvedBranch, msg => { fatalFailure = true; collector.EmitError(string.Empty, msg); }, msg => collector.EmitWarning(string.Empty, msg), ctx); @@ -770,8 +796,8 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments foreach (var entry in entries) byName[entry.FileName] = entry.Content; - _logger.LogInformation("Sourced {Count} changelog entr(ies) from the CDN for repo {Repo}", - byName.Count, repo); + _logger.LogInformation("Sourced {Count} changelog entr(ies) from the CDN for {Pool}", + byName.Count, $"{resolvedOrg}/{repo}/{resolvedBranch}"); return byName.Select(kv => (kv.Key, kv.Value)).ToList(); } diff --git a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs index 7451f965b..dd0e37180 100644 --- a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs +++ b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs @@ -31,18 +31,28 @@ public record ChangelogUploadArguments public string? Directory { get; init; } /// - /// Authoring repository identifier used to scope changelog-entry keys (changelog/{repo}/{file}). - /// Required for uploads; unused for bundle uploads (which are - /// product-scoped from the bundle YAML). Resolved by the CLI via the precedence - /// --repo > bundle.repo > git remote origin. + /// Authoring repository identifier used to scope changelog-entry keys + /// (changelog/{org}/{repo}/{branch}/{file}). Required for + /// uploads; unused for bundle uploads (which are product-scoped from the bundle YAML). Resolved by the + /// CLI via the precedence --repo > bundle.repo > git remote origin. /// public string? Repo { get; init; } /// - /// GitHub owner, accepted for parity with other changelog commands. Not part of the S3 key - /// (entry keys are scoped by repo only); retained for diagnostics and future use. + /// GitHub owner (org), the first segment of changelog-entry keys + /// (changelog/{org}/{repo}/{branch}/{file}). Required for + /// uploads; unused for bundle uploads. Resolved by the CLI via the precedence + /// --owner > bundle.owner > git remote origin. /// public string? Owner { get; init; } + + /// + /// Branch segment of changelog-entry keys (changelog/{org}/{repo}/{branch}/{file}), stored + /// verbatim so any / in the branch become real key separators (e.g. feature/foo). + /// Required for uploads; unused for bundle uploads. Resolved by the + /// CLI via the precedence --branch > the current git branch. + /// + public string? Branch { get; init; } } public partial class ChangelogUploadService( @@ -61,11 +71,16 @@ public partial class ChangelogUploadService( [GeneratedRegex(@"^[a-zA-Z0-9_-]+$")] private static partial Regex ProductNameRegex(); - // Authoring repo identifier used as a single S3 path segment (changelog/{repo}/{file}). - // Same character class as product names; "." / ".." are additionally rejected to prevent traversal. + // Authoring repo identifier used as a single S3 path segment (changelog/{org}/{repo}/{branch}/{file}). + // Same character class as product names but also allows "."; "." / ".." are rejected to prevent traversal. [GeneratedRegex(@"^[a-zA-Z0-9._-]+$")] private static partial Regex RepoNameRegex(); + // GitHub owner (org) login used as the first changelog key segment (changelog/{org}/...). GitHub logins + // are ASCII alphanumerics with single hyphens; "." is intentionally excluded. + [GeneratedRegex(@"^[a-zA-Z0-9-]+$")] + private static partial Regex OrgNameRegex(); + public async Task Upload(IDiagnosticsCollector collector, ChangelogUploadArguments args, Cancel ctx) { if (args.Target == UploadTargetKind.Elasticsearch) @@ -89,7 +104,7 @@ public async Task Upload(IDiagnosticsCollector collector, ChangelogUploadA var targets = args.ArtifactType == ArtifactType.Bundle ? DiscoverBundleUploadTargets(collector, directory) - : DiscoverUploadTargets(collector, directory, args.Repo); + : DiscoverUploadTargets(collector, directory, args.Owner, args.Repo, args.Branch); // Entry uploads abort (rather than no-op) when the repo cannot be resolved: the keys would be // unscoped and a silent skip would look like "nothing to upload". @@ -153,11 +168,19 @@ private async Task RefreshRegistries( } } - internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector collector, string changelogDir, string? repo) + internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector collector, string changelogDir, string? org, string? repo, string? branch) { - // Option AD: entries live once, under the authoring repo's pool — independent of which products - // later consume them. The repo must be resolvable (CLI --repo > bundle.repo > git remote); a - // missing/invalid value is fatal because every entry key derives from it. + // Option AD: entries live once, under the authoring org/repo/branch pool — independent of which + // products later consume them. Org, repo, and branch must all resolve (CLI flags > bundle config > + // git); a missing/invalid value is fatal because every entry key derives from them. + if (string.IsNullOrWhiteSpace(org) || !OrgNameRegex().IsMatch(org)) + { + collector.EmitError(string.Empty, + $"A valid GitHub owner is required to upload changelog entries (resolved: \"{org ?? ""}\"). " + + "Set --owner, bundle.owner in changelog.yml, or run inside a checkout with a github.com origin remote."); + return []; + } + if (string.IsNullOrWhiteSpace(repo) || !RepoNameRegex().IsMatch(repo) || repo is "." or "..") { collector.EmitError(string.Empty, @@ -166,6 +189,14 @@ internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector return []; } + if (string.IsNullOrWhiteSpace(branch) || !IsValidBranch(branch)) + { + collector.EmitError(string.Empty, + $"A valid branch is required to upload changelog entries (resolved: \"{branch ?? ""}\"). " + + "Set --branch or run inside a checkout with a current branch."); + return []; + } + var rootDir = _fileSystem.DirectoryInfo.New(changelogDir); var yamlFiles = _fileSystem.Directory.GetFiles(changelogDir, "*.yaml", SearchOption.TopDirectoryOnly) @@ -184,13 +215,26 @@ internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector } var fileName = _fileSystem.Path.GetFileName(filePath); - var s3Key = $"changelog/{repo}/{fileName}"; + var s3Key = $"changelog/{org}/{repo}/{branch}/{fileName}"; targets.Add(new UploadTarget(filePath, s3Key)); } return targets; } + // Validates a branch used verbatim as one-or-more changelog key segments + // (changelog/{org}/{repo}/{branch}/…). Each "/"-delimited part uses the repo-name class; empty parts + // and "." / ".." are rejected so slashes stay meaningful without enabling traversal. + private static bool IsValidBranch(string branch) + { + foreach (var part in branch.Split('/')) + { + if (part.Length == 0 || part is "." or ".." || !RepoNameRegex().IsMatch(part)) + return false; + } + return true; + } + internal IReadOnlyList DiscoverBundleUploadTargets(IDiagnosticsCollector collector, string bundleDir) { var rootDir = _fileSystem.DirectoryInfo.New(bundleDir); diff --git a/src/services/Elastic.Changelog/Uploading/Registry.cs b/src/services/Elastic.Changelog/Uploading/Registry.cs index 88eac25ca..376810e1d 100644 --- a/src/services/Elastic.Changelog/Uploading/Registry.cs +++ b/src/services/Elastic.Changelog/Uploading/Registry.cs @@ -13,8 +13,8 @@ namespace Elastic.Changelog.Uploading; /// /// /// Stored at bundle/{product}/registry.json (bundle index) or -/// changelog/{repo}/registry.json (changelog-entry index) in the changelog bundles bucket. -/// The scrubber Lambda mirrors it verbatim to the public bucket (pass-through). +/// changelog/{org}/{repo}/{branch}/registry.json (changelog-entry index) in the changelog bundles +/// bucket. The scrubber Lambda mirrors it verbatim to the public bucket (pass-through). /// public sealed record Registry { @@ -25,8 +25,8 @@ public sealed record Registry /// /// Grouping identifier: the product for a bundle index (bundle/{product}/…) or the - /// authoring repo for a changelog-entry index (changelog/{repo}/…) — i.e. the second - /// path segment of the S3 key. Informational only; consumers locate the manifest by key. + /// {org}/{repo}/{branch} prefix for a changelog-entry index + /// (changelog/{org}/{repo}/{branch}/…). Informational only; consumers locate the manifest by key. /// public required string Product { get; init; } @@ -50,7 +50,7 @@ public sealed record RegistryBundle /// /// Bundle file name (e.g. 9.3.0.yaml or cloud-2025-11.yaml), /// resolved at bundle/{product}/{file}. For changelog-entry indexes this is the entry - /// file name resolved at changelog/{repo}/{file}. + /// file name resolved at changelog/{org}/{repo}/{branch}/{file}. /// public required string File { get; init; } diff --git a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs index 21dd9305c..30d3ae651 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs @@ -23,7 +23,7 @@ internal enum RegistryScope /// The bundle index at bundle/{product}/registry.json, listing scrubbed bundle files. Bundle, - /// The changelog-entry index at changelog/{repo}/registry.json, listing individual entry files. + /// The changelog-entry index at changelog/{org}/{repo}/{branch}/registry.json, listing individual entry files. Changelog } @@ -31,9 +31,10 @@ internal enum RegistryScope /// Refreshes a registry.json manifest in the private bucket after an upload run. /// Depending on this is either the bundle index /// (bundle/{product}/registry.json) or the changelog-entry index -/// (changelog/{repo}/registry.json). Each grouping (product or repo) 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 group cannot clobber each other). +/// (changelog/{org}/{repo}/{branch}/registry.json). Each grouping (product, or org/repo/branch) +/// 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 group cannot +/// clobber each other). /// internal sealed class RegistryBuilder( ILoggerFactory logFactory, @@ -71,8 +72,8 @@ public async Task RefreshAsync( RegistryScope scope = RegistryScope.Bundle) { // Each upload target carries an artifact-root S3 key — "bundle/{product}/{file}" or - // "changelog/{repo}/{file}". Group by the scope's second segment (product for bundles, repo for - // entries) so we produce one manifest per affected group. + // "changelog/{org}/{repo}/{branch}/{file}". Group by the scope's key (product for bundles, the + // {org}/{repo}/{branch} prefix for entries) so we produce one manifest per affected group. var byProduct = uploadTargets .Select(t => (Target: t, Product: ExtractGroupKey(t.S3Key, scope))) .Where(x => x.Product is not null) @@ -111,7 +112,7 @@ public async Task RefreshAsync( return new RefreshResult(updated, unchanged, failed); } - /// Extracts the grouping segment (product for bundle/{product}/…, repo for changelog/{repo}/…) from an artifact-root S3 key, or null. + /// Extracts the grouping key (product for bundle/{product}/…, {org}/{repo}/{branch} for changelog/{org}/{repo}/{branch}/…) from an artifact-root S3 key, or null. private static string? ExtractGroupKey(string s3Key, RegistryScope scope) { var prefix = scope == RegistryScope.Changelog ? "changelog/" : "bundle/"; @@ -119,10 +120,36 @@ public async Task RefreshAsync( return null; var rest = s3Key.AsSpan(prefix.Length); - var slash = rest.IndexOf('/'); - if (slash <= 0) + + // Bundles group by the single product segment (bundle/{product}/{file}). Changelog entries group by + // the whole {org}/{repo}/{branch} prefix (changelog/{org}/{repo}/{branch...}/{file}), which can carry + // extra slashes when the branch itself contains them — so take everything before the final segment. + if (scope == RegistryScope.Bundle) + { + var slash = rest.IndexOf('/'); + return slash <= 0 ? null : rest[..slash].ToString(); + } + + var lastSlash = rest.LastIndexOf('/'); + if (lastSlash <= 0) return null; - return rest[..slash].ToString(); + var group = rest[..lastSlash]; + // Require at least org/repo/branch (3 segments) ahead of the file name. + return CountSegments(group) >= 3 ? group.ToString() : null; + } + + /// Counts the non-empty-delimited segments in a /-joined path span. + private static int CountSegments(ReadOnlySpan path) + { + if (path.IsEmpty) + return 0; + var count = 1; + foreach (var c in path) + { + if (c == '/') + count++; + } + return count; } /// The S3 key of the manifest for the given and grouping segment. diff --git a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs index b0b2567fc..4b8e9d14d 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs @@ -5,7 +5,7 @@ namespace Elastic.Changelog.Uploading; /// -/// Helpers for validating S3 object keys related to the per-product +/// Helpers for validating S3 object keys related to the per-group /// manifests (the bundle index and the changelog-entry index). /// public static class RegistryKey @@ -16,43 +16,58 @@ public static class RegistryKey /// /// Returns true when is a manifest of either artifact-root form - /// bundle/{product}/registry.json (the bundle index) or - /// changelog/{repo}/registry.json (the changelog-entry index). The bundle product - /// segment matches the producer's product class ([a-zA-Z0-9_-]+); the changelog repo - /// segment additionally allows . (GitHub repo names can contain dots). + /// bundle/{product}/registry.json (the bundle index, exactly one product segment) or + /// changelog/{org}/{repo}/{branch}/registry.json (the changelog-entry index, at least three + /// segments because the branch is stored verbatim and may itself contain /). The bundle product + /// segment matches the producer's product class ([a-zA-Z0-9_-]+); the changelog segments + /// additionally allow . (GitHub repo names and branches such as 8.x can contain dots). /// /// /// Used by the changelog scrubber Lambda to decide whether to pass an incoming /// *.json object through to the public bucket. Anything else (a bare - /// {x}/registry.json, a deeper nesting, or an unknown top-level prefix) is - /// rejected, which keeps arbitrary JSON out of the public surface. + /// {x}/registry.json, a changelog manifest shallower than org/repo/branch, or an unknown + /// top-level prefix) is rejected, which keeps arbitrary JSON out of the public surface. /// public static bool IsRegistry(string key) { if (string.IsNullOrEmpty(key)) return false; - return IsScopedRegistry(key, BundlePrefix) || IsScopedRegistry(key, ChangelogPrefix); + // Bundle index: exactly one product segment, no dots. Changelog-entry index: the + // {org}/{repo}/{branch} prefix — at least three segments, dots allowed (repo/branch may contain them). + return IsScopedRegistry(key, BundlePrefix, minSegments: 1, maxSegments: 1, allowDots: false) + || IsScopedRegistry(key, ChangelogPrefix, minSegments: 3, maxSegments: int.MaxValue, allowDots: true); } - private static bool IsScopedRegistry(string key, string prefix) + private static bool IsScopedRegistry(string key, string prefix, int minSegments, int maxSegments, bool allowDots) { if (!key.StartsWith(prefix, StringComparison.Ordinal) || !key.EndsWith(RegistrySuffix, StringComparison.Ordinal)) return false; - var segmentLength = key.Length - prefix.Length - RegistrySuffix.Length; - if (segmentLength <= 0) + var middleLength = key.Length - prefix.Length - RegistrySuffix.Length; + if (middleLength <= 0) return false; - // Bundle products never contain dots (producer class is [a-zA-Z0-9_-]+); only changelog repo - // segments may, so dots are accepted under changelog/ but not bundle/. - var allowDots = string.Equals(prefix, ChangelogPrefix, StringComparison.Ordinal); - return IsValidSegment(key.AsSpan(prefix.Length, segmentLength), allowDots: allowDots); + var middle = key.AsSpan(prefix.Length, middleLength); + var segments = 0; + var start = 0; + for (var i = 0; i <= middle.Length; i++) + { + if (i != middle.Length && middle[i] != '/') + continue; + + segments++; + if (segments > maxSegments || !IsValidSegment(middle[start..i], allowDots)) + return false; + start = i + 1; + } + + return segments >= minSegments; } private static bool IsValidSegment(ReadOnlySpan segment, bool allowDots) { - if (segment.IsEmpty || segment.Contains('/') || segment is "." or "..") + if (segment.IsEmpty || segment is "." or "..") return false; foreach (var c in segment) diff --git a/src/tooling/docs-builder/Commands/ChangelogCommand.cs b/src/tooling/docs-builder/Commands/ChangelogCommand.cs index de913cd79..dea891a15 100644 --- a/src/tooling/docs-builder/Commands/ChangelogCommand.cs +++ b/src/tooling/docs-builder/Commands/ChangelogCommand.cs @@ -561,6 +561,7 @@ async static (s, collector, state, ctx) => await s.CreateChangelog(collector, st /// Optional: Output path for the bundled changelog. Can be either (1) a directory path, in which case 'changelog-bundle.yaml' is created in that directory, or (2) a file path ending in .yml or .yaml. Uses config bundle.output_directory or defaults to 'changelog-bundle.yaml' in the input directory /// Optional: Explicitly set the products array in the output file in format "product target lifecycle, ...". Overrides any values from changelogs. /// GitHub repository owner, which is used when PRs or issues are specified as numbers or when using --release-version. Falls back to bundle.owner in changelog.yml when not specified. If that value is also absent, "elastic" is used. + /// Branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/...) entries are sourced from. Falls back to bundle.branch in changelog.yml, then "main". /// Filter by pull request URLs (comma-separated), or a path to a newline-delimited file containing fully-qualified GitHub PR URLs. Can be specified multiple times. /// GitHub repository name, which is used when PRs or issues are specified as numbers or when using --release-version. Falls back to bundle.repo in changelog.yml when not specified. If that value is also absent, the product ID is used. /// A URL or file path to a promotion report. Extracts PR URLs and uses them as the filter. @@ -585,6 +586,7 @@ public async Task Bundle( [ArgumentParser(typeof(ProductInfoParser))] ProductArgumentList? outputProducts = null, string[]? issues = null, string? owner = null, + string? branch = null, bool plan = false, string[]? prs = null, string? releaseVersion = null, @@ -896,6 +898,7 @@ public async Task Bundle( Issues = allIssues.Count > 0 ? allIssues.ToArray() : null, Owner = owner, Repo = repo, + Branch = branch, Profile = profile, ProfileArgument = profileArg, ProfileReport = isProfileMode ? profileReport : null, @@ -1575,10 +1578,12 @@ private string ApplyChangelogInitBundleRepoSeed(string content, string? ownerCli /// /// Uses content-hash–based incremental transfer — only changed files are uploaded. /// - /// Changelog entries are uploaded once under changelog/{repo}/{file}, keyed by the authoring - /// repository (resolved from --repo, then bundle.repo in changelog.yml, then the git - /// remote origin). Bundles are uploaded under bundle/{product}/{file}, product-scoped from - /// the bundle YAML, and do not require a repo. + /// Changelog entries are uploaded once under changelog/{org}/{repo}/{branch}/{file}, keyed by the + /// authoring owner (--owner > bundle.owner > git remote), repository (--repo + /// > bundle.repo > git remote), and branch (--branch > the current checkout's + /// branch). The branch is stored verbatim, so a branch's / become real key separators. Bundles + /// are uploaded under bundle/{product}/{file}, product-scoped from the bundle YAML, and do not + /// require an owner/repo/branch. /// /// /// Artifact type to upload: 'changelog' (individual entries) or 'bundle' (consolidated bundles). @@ -1586,8 +1591,9 @@ private string ApplyChangelogInitBundleRepoSeed(string content, string? ownerCli /// S3 bucket name (required when target is 's3'). /// Path to changelog.yml configuration file. Defaults to docs/changelog.yml. /// Override changelog directory instead of reading it from config. - /// GitHub repository name used to scope uploaded changelog entry keys (changelog/{repo}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads. - /// GitHub repository owner, accepted for parity with other changelog commands. Falls back to bundle.owner in changelog.yml, then the git remote origin. Not part of the S3 key. + /// GitHub repository name, the second segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads. + /// GitHub owner (org), the first segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.owner in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads. + /// Branch, the third segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...), stored verbatim. Falls back to the current checkout's branch. Required for changelog uploads; ignored for bundle uploads. [NoOptionsInjection] public async Task Upload( string artifactType, @@ -1597,6 +1603,7 @@ public async Task Upload( [ExpandUserProfile, RejectSymbolicLinks] DirectoryInfo? directory = null, string? repo = null, string? owner = null, + string? branch = null, CancellationToken ct = default ) { @@ -1622,9 +1629,10 @@ public async Task Upload( var resolvedDirectory = directory != null ? directory?.FullName : null; var resolvedConfig = config != null ? config?.FullName : null; - // Resolve the authoring repo for entry keys: --repo > bundle.repo (changelog.yml) > git remote - // origin. Reduced to a single path segment (owner/repo -> repo) for the changelog/{repo}/ key. - var (resolvedRepo, resolvedOwner) = await ResolveUploadRepoOwner(repo, owner, resolvedConfig, resolvedDirectory, ctx); + // Resolve the authoring owner/repo/branch for entry keys: CLI flags > bundle.{owner,repo} + // (changelog.yml) > git. The repo is reduced to a single path segment (owner/repo -> repo) for the + // changelog/{org}/{repo}/{branch}/ key. + var (resolvedRepo, resolvedOwner, resolvedBranch) = await ResolveUploadRepoOwnerBranch(repo, owner, branch, resolvedConfig, resolvedDirectory, ctx); await using var serviceInvoker = new ServiceInvoker(collector); var service = new ChangelogUploadService(logFactory, configurationContext); @@ -1636,7 +1644,8 @@ public async Task Upload( Config = resolvedConfig, Directory = resolvedDirectory, Repo = resolvedRepo, - Owner = resolvedOwner + Owner = resolvedOwner, + Branch = resolvedBranch }; serviceInvoker.AddCommand(service, args, static async (s, c, state, ct) => await s.Upload(c, state, ct) @@ -1644,15 +1653,15 @@ static async (s, c, state, ct) => await s.Upload(c, state, ct) return await serviceInvoker.InvokeAsync(ctx); } - /// Resolves the authoring repo/owner for uploads (--repo > bundle.repo > git remote), reducing the repo to a single path segment. - private async Task<(string? Repo, string? Owner)> ResolveUploadRepoOwner(string? repoCli, string? ownerCli, string? configPath, string? uploadDirectory, CancellationToken ctx) + /// Resolves the authoring repo/owner/branch for uploads (CLI flags > bundle.{repo,owner} > git), reducing the repo to a single path segment. + private async Task<(string? Repo, string? Owner, string? Branch)> ResolveUploadRepoOwnerBranch(string? repoCli, string? ownerCli, string? branchCli, string? configPath, string? uploadDirectory, CancellationToken ctx) { var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem) .LoadChangelogConfiguration(collector, configPath, ctx); - // Anchor the git-remote fallback to the upload source (config file or changelog directory), not - // the process cwd, so an out-of-tree --config/--directory resolves the right origin. Both values - // are absolute (FileInfo/DirectoryInfo FullName) when present. + // Anchor the git fallbacks to the upload source (config file or changelog directory), not the + // process cwd, so an out-of-tree --config/--directory resolves the right origin and branch. Both + // values are absolute (FileInfo/DirectoryInfo FullName) when present. string? anchor = null; if (!string.IsNullOrWhiteSpace(configPath)) { @@ -1673,6 +1682,15 @@ static async (s, c, state, ct) => await s.Upload(c, state, ct) var resolvedRepo = !string.IsNullOrWhiteSpace(repoCli) ? repoCli : (bundleConfig?.Bundle?.Repo ?? gitRepo); var resolvedOwner = ownerCli ?? bundleConfig?.Bundle?.Owner ?? gitOwner; + // The producer branch is the branch being published: --branch, else the current checkout's branch. + // bundle.branch is intentionally not consulted here — it selects which pool to read when bundling. + var resolvedBranch = branchCli; + if (string.IsNullOrWhiteSpace(resolvedBranch)) + { + var checkout = GitCheckoutInformationFactory.Create(_fileSystem.DirectoryInfo.New(anchor), _fileSystem); + resolvedBranch = checkout.Branch; + } + if (!string.IsNullOrWhiteSpace(resolvedRepo)) { var slash = resolvedRepo.LastIndexOf('/'); @@ -1680,7 +1698,7 @@ static async (s, c, state, ct) => await s.Upload(c, state, ct) resolvedRepo = resolvedRepo[(slash + 1)..]; } - return (resolvedRepo, resolvedOwner); + return (resolvedRepo, resolvedOwner, resolvedBranch); } /// diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs index bdd1f4c1a..73a59e4c5 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs @@ -90,8 +90,8 @@ public async Task OptionMode_RepoResolvable_SourcesAllEntriesFromRepoPoolOnCdn() result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); Collector.Errors.Should().Be(0); - // Entries are sourced from the authoring repo's pool: changelog/{repo}/... - handler.RequestedPaths.Should().Contain("/changelog/elasticsearch/registry.json"); + // Entries are sourced from the authoring pool, with org/branch defaulting: changelog/{org}/{repo}/{branch}/... + handler.RequestedPaths.Should().Contain("/changelog/elastic/elasticsearch/main/registry.json"); var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); bundle.Should().Contain("Alpha"); @@ -99,6 +99,31 @@ public async Task OptionMode_RepoResolvable_SourcesAllEntriesFromRepoPoolOnCdn() bundle.Should().Contain("name: 1-alpha.yaml"); } + [Fact] + public async Task OptionMode_OwnerAndBranchOverride_SourcesFromThatPoolOnCdn() + { + // Explicit owner/branch select a specific pool; the branch is stored verbatim (dots kept). + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher(Output, handler)); + var output = OutputPath(); + + var input = new BundleChangelogsArguments + { + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = output, + Owner = "acme-corp", + Repo = "elasticsearch", + Branch = "8.x", + 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); + handler.RequestedPaths.Should().Contain("/changelog/acme-corp/elasticsearch/8.x/registry.json"); + } + [Fact] public async Task OptionMode_NoResolvableRepo_FallsBackToLocal() { diff --git a/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs index ef4e73b11..93997c9bf 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs @@ -18,7 +18,7 @@ namespace Elastic.Changelog.Tests.Changelogs; /// anonymized repo and product names only — never the real org/repo/product IDs. /// /// It verifies the artifact-root (Option AD) behaviour: entries are sourced once from the authoring -/// repo's pool (changelog/{repo}/...), and the profile bundle that results is sound — resolved, +/// pool (changelog/{org}/{repo}/{branch}/...), and the profile bundle that results is sound — resolved, /// link-scrubbed against the allowlist, with the docs entry excluded and no auto-populated release date. /// /// The authoring repo is an anonymized test name (widget) that deliberately differs from the @@ -148,8 +148,9 @@ public async Task MonthlyProfile_SourcesFromRepoPool_ProducesSoundBundle() result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); Collector.Errors.Should().Be(0); - // Entries are sourced once from the authoring repo's pool, not from any product-scoped path. - handler.RequestedPaths.Should().Contain($"/changelog/{AuthoringRepo}/registry.json"); + // Entries are sourced once from the authoring pool (changelog/{org}/{repo}/{branch}/...), not from + // any product-scoped path. Owner comes from bundle.owner; branch defaults to "main". + handler.RequestedPaths.Should().Contain($"/changelog/elastic/{AuthoringRepo}/main/registry.json"); handler.RequestedPaths.Should().NotContain(p => p.Contains("/cloud-hosted/changelog/", StringComparison.Ordinal)); var outputFiles = FileSystem.Directory.GetFiles(outputDir, "*.yaml"); diff --git a/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs index 9b8db13e4..929f697e2 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs @@ -534,7 +534,7 @@ public void TryApplyChangelogEntry_NullFields_PreservesNulls() public void TryApplyChangelogEntry_BarePrNumberWithoutDefaultRepo_KeptWithWarning() { // The scrubber Lambda calls TryApplyChangelogEntry with defaultRepo=null because per-entry - // YAMLs uploaded under changelog/{repo}/*.yaml carry no embedded repo context. A bare + // YAMLs uploaded under changelog/{org}/{repo}/{branch}/*.yaml carry no embedded repo context. A bare // numeric PR ref ("155500") must be tolerated rather than failing the whole entry — the // reference carries no repo identity so it cannot leak a private link, and downstream // rendering supplies the owner/repo from runtime context. diff --git a/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs b/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs index f737ea405..30424e9c7 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs @@ -48,7 +48,7 @@ private string AddChangelog(string fileName, string yaml) } [Fact] - public void DiscoverUploadTargets_SingleEntry_MapsToRepoScopedKey() + public void DiscoverUploadTargets_SingleEntry_MapsToPoolScopedKey() { // language=yaml var path = AddChangelog("entry.yaml", """ @@ -61,11 +61,53 @@ public void DiscoverUploadTargets_SingleEntry_MapsToRepoScopedKey() - "100" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); targets.Should().HaveCount(1); targets[0].LocalPath.Should().Be(path); - targets[0].S3Key.Should().Be("changelog/elasticsearch/entry.yaml"); + targets[0].S3Key.Should().Be("changelog/elastic/elasticsearch/main/entry.yaml"); + _collector.Errors.Should().Be(0); + } + + [Theory] + // Branch stored verbatim: dots (release trains) and slashes (feature branches) become real key segments. + [InlineData("8.x", "changelog/elastic/elasticsearch/8.x/entry.yaml")] + [InlineData("9.0", "changelog/elastic/elasticsearch/9.0/entry.yaml")] + [InlineData("feature/foo", "changelog/elastic/elasticsearch/feature/foo/entry.yaml")] + [InlineData("release/8.x", "changelog/elastic/elasticsearch/release/8.x/entry.yaml")] + public void DiscoverUploadTargets_BranchWithDotsOrSlashes_MapsVerbatim(string branch, string expectedKey) + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", branch); + + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be(expectedKey); + _collector.Errors.Should().Be(0); + } + + [Fact] + public void DiscoverUploadTargets_ExternalOrg_MapsToPoolScopedKey() + { + // An acquired company keeping its own GitHub org still gets a faithful pool. + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: widgets + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "acme-corp", "widgets", "main"); + + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be("changelog/acme-corp/widgets/main/entry.yaml"); _collector.Errors.Should().Be(0); } @@ -87,10 +129,10 @@ public void DiscoverUploadTargets_EntryWithMultipleProducts_StillSingleRepoKey() - "200" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "kibana"); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "kibana", "main"); targets.Should().ContainSingle(); - targets[0].S3Key.Should().Be("changelog/kibana/fix.yaml"); + targets[0].S3Key.Should().Be("changelog/elastic/kibana/main/fix.yaml"); } [Fact] @@ -106,10 +148,10 @@ public void DiscoverUploadTargets_EntryWithNoProducts_StillUploaded() - "400" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); targets.Should().ContainSingle(); - targets[0].S3Key.Should().Be("changelog/elasticsearch/noproducts.yaml"); + targets[0].S3Key.Should().Be("changelog/elastic/elasticsearch/main/noproducts.yaml"); _collector.Errors.Should().Be(0); } @@ -124,7 +166,41 @@ public void DiscoverUploadTargets_MissingRepo_EmitsErrorAndReturnsEmpty() - product: elasticsearch """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, repo: null); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", repo: null, "main"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); + } + + [Fact] + public void DiscoverUploadTargets_MissingOwner_EmitsErrorAndReturnsEmpty() + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, org: null, "elasticsearch", "main"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); + } + + [Fact] + public void DiscoverUploadTargets_MissingBranch_EmitsErrorAndReturnsEmpty() + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", branch: null); targets.Should().BeEmpty(); _collector.Errors.Should().BeGreaterThan(0); @@ -145,7 +221,50 @@ public void DiscoverUploadTargets_InvalidRepo_EmitsError(string repo) - product: elasticsearch """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, repo); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", repo, "main"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); + } + + [Theory] + // Org is a GitHub login: no dots, no slashes, no spaces. + [InlineData("bad org")] + [InlineData("acme.corp")] + [InlineData("acme/corp")] + public void DiscoverUploadTargets_InvalidOrg_EmitsError(string org) + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, org, "elasticsearch", "main"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); + } + + [Theory] + // Branch segments reject traversal, empty parts, and out-of-class characters. + [InlineData("bad branch")] + [InlineData("..")] + [InlineData("feature/..")] + [InlineData("feature//foo")] + public void DiscoverUploadTargets_InvalidBranch_EmitsError(string branch) + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", branch); targets.Should().BeEmpty(); _collector.Errors.Should().BeGreaterThan(0); @@ -154,7 +273,7 @@ public void DiscoverUploadTargets_InvalidRepo_EmitsError(string repo) [Fact] public void DiscoverUploadTargets_EmptyDirectory_ReturnsEmpty() { - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); targets.Should().BeEmpty(); _collector.Errors.Should().Be(0); @@ -182,11 +301,11 @@ public void DiscoverUploadTargets_MultipleFiles_DiscoversAllUnderRepo() - "2" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elasticsearch"); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "changelog/elasticsearch/first.yaml"); - targets.Should().Contain(t => t.S3Key == "changelog/elasticsearch/second.yaml"); + targets.Should().Contain(t => t.S3Key == "changelog/elastic/elasticsearch/main/first.yaml"); + targets.Should().Contain(t => t.S3Key == "changelog/elastic/elasticsearch/main/second.yaml"); } [Theory] @@ -206,10 +325,10 @@ public void DiscoverUploadTargets_RepoWithHyphensDotsUnderscores_Accepted(string - "600" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, repo); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", repo, "main"); targets.Should().ContainSingle(); - targets[0].S3Key.Should().Be($"changelog/{repo}/entry.yaml"); + targets[0].S3Key.Should().Be($"changelog/elastic/{repo}/main/entry.yaml"); _collector.Errors.Should().Be(0); _collector.Warnings.Should().Be(0); } @@ -240,7 +359,9 @@ public async Task Upload_WithValidChangelogs_UploadsToS3() Target = UploadTargetKind.S3, S3BucketName = "test-bucket", Directory = _changelogDir, - Repo = "elasticsearch" + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -249,7 +370,7 @@ public async Task Upload_WithValidChangelogs_UploadsToS3() _collector.Errors.Should().Be(0); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "changelog/elasticsearch/entry.yaml" && r.BucketName == "test-bucket"), + A.That.Matches(r => r.Key == "changelog/elastic/elasticsearch/main/entry.yaml" && r.BucketName == "test-bucket"), A._ )).MustHaveHappenedOnceExactly(); } @@ -273,7 +394,9 @@ public async Task Upload_ChangelogWithoutRepo_FailsWithoutS3Calls() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Owner = "elastic", + Branch = "main" // Repo intentionally unset. }; var ct = TestContext.Current.CancellationToken; @@ -295,7 +418,9 @@ public async Task Upload_EmptyDirectory_ReturnsTrue() Target = UploadTargetKind.S3, S3BucketName = "test-bucket", Directory = _changelogDir, - Repo = "elasticsearch" + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -332,7 +457,9 @@ public async Task Upload_WithFailedUpload_ReturnsFalseAndEmitsError() Target = UploadTargetKind.S3, S3BucketName = "test-bucket", Directory = _changelogDir, - Repo = "elasticsearch" + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -567,16 +694,18 @@ public async Task Upload_ChangelogArtifactType_RefreshesRepoScopedRegistry() Target = UploadTargetKind.S3, S3BucketName = "test-bucket", Directory = _changelogDir, - Repo = "elasticsearch" + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); result.Should().BeTrue(); - // Changelog uploads refresh the repo-scoped entry index, not a bundle index. + // Changelog uploads refresh the pool-scoped entry index, not a bundle index. A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "changelog/elasticsearch/registry.json"), + A.That.Matches(r => r.Key == "changelog/elastic/elasticsearch/main/registry.json"), A._ )).MustHaveHappenedOnceExactly(); diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs index 01fe9de97..39e774432 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs @@ -408,7 +408,7 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() - product: elasticsearch target: 9.3.0 """)); - var targets = new List { new(path, "changelog/elasticsearch/1-feature.yaml") }; + var targets = new List { new(path, "changelog/elastic/elasticsearch/main/1-feature.yaml") }; A.CallTo(() => _s3Client.GetObjectAsync(A._, A._)) .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); @@ -416,10 +416,11 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() result.Updated.Should().Be(1); _puts.Should().ContainSingle(); - _puts[0].Key.Should().Be("changelog/elasticsearch/registry.json"); + _puts[0].Key.Should().Be("changelog/elastic/elasticsearch/main/registry.json"); var manifest = Deserialize(_puts[0].ContentBody); - manifest.Product.Should().Be("elasticsearch"); + // The grouping identifier for the entry index is the {org}/{repo}/{branch} prefix. + manifest.Product.Should().Be("elastic/elasticsearch/main"); 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. @@ -427,6 +428,34 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() manifest.Bundles[0].ETag.Should().NotBeNullOrEmpty(); } + [Fact] + public async Task Refresh_ChangelogScope_BranchWithSlashes_GroupsByFullPoolPrefix() + { + var path = _mockFileSystem.Path.Join(_bundleDir, "2-feature.yaml"); + // language=yaml + _mockFileSystem.AddFile(path, new MockFileData(""" + title: Sample on a feature branch + type: enhancement + products: + - product: elasticsearch + """)); + // The branch "feature/foo" contributes two key segments; the registry must live at the pool root. + var targets = new List { new(path, "changelog/elastic/elasticsearch/feature/foo/2-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("changelog/elastic/elasticsearch/feature/foo/registry.json"); + + var manifest = Deserialize(_puts[0].ContentBody); + manifest.Product.Should().Be("elastic/elasticsearch/feature/foo"); + manifest.Bundles.Should().ContainSingle(); + manifest.Bundles[0].File.Should().Be("2-feature.yaml"); + } + 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 9ca9eac5a..2e253c90d 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs @@ -10,19 +10,25 @@ namespace Elastic.Changelog.Tests.Uploading; public class RegistryKeyTests { [Theory] - // Bundle index (artifact-root): bundle/{product}/registry.json + // Bundle index (artifact-root): bundle/{product}/registry.json — exactly one product segment. [InlineData("bundle/elasticsearch/registry.json")] [InlineData("bundle/kibana/registry.json")] [InlineData("bundle/elastic-agent/registry.json")] [InlineData("bundle/cloud_hosted/registry.json")] [InlineData("bundle/cloud-serverless/registry.json")] [InlineData("bundle/a/registry.json")] - // Changelog-entry index (artifact-root): changelog/{repo}/registry.json - [InlineData("changelog/elasticsearch/registry.json")] - [InlineData("changelog/kibana/registry.json")] - [InlineData("changelog/cloud/registry.json")] - // Repo segments may contain dots (e.g. apm-agent-dotnet, elasticsearch-net). - [InlineData("changelog/apm.agent/registry.json")] + // Changelog-entry index (artifact-root): changelog/{org}/{repo}/{branch}/registry.json. + [InlineData("changelog/elastic/elasticsearch/main/registry.json")] + [InlineData("changelog/elastic/kibana/master/registry.json")] + // External org (e.g. an acquired company keeping its own GitHub org). + [InlineData("changelog/acme-corp/widgets/main/registry.json")] + // Repo and branch segments may contain dots (e.g. apm-agent-dotnet, branch 8.x). + [InlineData("changelog/elastic/apm.agent/main/registry.json")] + [InlineData("changelog/elastic/elasticsearch/8.x/registry.json")] + [InlineData("changelog/elastic/kibana/9.0/registry.json")] + // Branch stored verbatim: a branch's own '/' become additional, valid key segments. + [InlineData("changelog/elastic/kibana/feature/foo/registry.json")] + [InlineData("changelog/elastic/kibana/release/8.x/registry.json")] public void IsRegistry_ValidArtifactRootKeys_ReturnsTrue(string key) => RegistryKey.IsRegistry(key).Should().BeTrue(); @@ -30,28 +36,36 @@ public void IsRegistry_ValidArtifactRootKeys_ReturnsTrue(string key) => [InlineData("")] [InlineData("registry.json")] [InlineData("/registry.json")] - // Old product-first layout is no longer a valid manifest key. + // Old product-first and single-segment changelog layouts are no longer valid manifest keys. [InlineData("elasticsearch/registry.json")] [InlineData("elasticsearch/changelog/registry.json")] + [InlineData("changelog/elasticsearch/registry.json")] + // Changelog manifests shallower than org/repo/branch (3 segments) are rejected. + [InlineData("changelog/elastic/elasticsearch/registry.json")] // Missing/empty middle segment. [InlineData("bundle/registry.json")] [InlineData("changelog/registry.json")] [InlineData("bundle//registry.json")] + [InlineData("changelog/elastic//main/registry.json")] // Unknown top-level prefix. - [InlineData("entries/elasticsearch/registry.json")] + [InlineData("entries/elastic/elasticsearch/main/registry.json")] [InlineData("elasticsearch/bundle/registry.json")] - // Dots are allowed only for changelog repo segments, never for bundle product segments + // Dots are allowed only for changelog segments, never for bundle product segments // (producers validate products as [a-zA-Z0-9_-]+). [InlineData("bundle/foo.bar/registry.json")] // Wrong extension. [InlineData("bundle/elasticsearch/registry.yaml")] - [InlineData("changelog/elasticsearch/registry.yaml")] - // Deeper nesting / traversal in the middle segment. + [InlineData("changelog/elastic/elasticsearch/main/registry.yaml")] + // Deeper nesting is rejected for bundles (must stay single-segment). [InlineData("bundle/elastic/search/registry.json")] - [InlineData("changelog/elastic/search/registry.json")] + // Traversal anywhere in the middle segments. [InlineData("bundle/../registry.json")] [InlineData("changelog/../registry.json")] + [InlineData("changelog/elastic/../main/registry.json")] + [InlineData("changelog/elastic/elasticsearch/../registry.json")] + // Spaces (and other out-of-class characters) are rejected. [InlineData("bundle/elastic search/registry.json")] + [InlineData("changelog/elastic/elastic search/main/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 index 109ae13fc..e1e9f7061 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs @@ -43,15 +43,35 @@ public async Task FetchAsync_HappyPath_ReturnsAllEntriesFromRegistry() var (errors, warnings, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", 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")); - // Artifact-root layout: entries and their registry live under changelog/{repo}/... - handler.RequestedPaths.Should().Contain("/changelog/elasticsearch/registry.json"); - handler.RequestedPaths.Should().Contain(p => p.EndsWith("/changelog/elasticsearch/1-a.yaml", StringComparison.Ordinal)); + // Artifact-root layout: entries and their registry live under changelog/{org}/{repo}/{branch}/... + handler.RequestedPaths.Should().Contain("/changelog/elastic/elasticsearch/main/registry.json"); + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/changelog/elastic/elasticsearch/main/1-a.yaml", StringComparison.Ordinal)); + } + + [Fact] + public async Task FetchAsync_BranchWithSlashes_KeepsBranchSeparatorsInPath() + { + var handler = new StubHandler(req => + req.RequestUri!.AbsolutePath.EndsWith("/registry.json", StringComparison.Ordinal) + ? Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elastic/elasticsearch/feature/foo", "bundles": [ { "file": "1-a.yaml" } ] }""") + : Yaml(SampleEntry)); + var (errors, warnings, emitError, emitWarning) = Diagnostics(); + + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "feature/foo", emitError, emitWarning, TestContext.Current.CancellationToken); + + errors.Should().BeEmpty(); + warnings.Should().BeEmpty(); + entries.Select(e => e.FileName).Should().BeEquivalentTo("1-a.yaml"); + // The branch's '/' stays a real path separator (not percent-encoded into one segment). + handler.RequestedPaths.Should().Contain("/changelog/elastic/elasticsearch/feature/foo/registry.json"); + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/changelog/elastic/elasticsearch/feature/foo/1-a.yaml", StringComparison.Ordinal)); } [Fact] @@ -61,7 +81,7 @@ public async Task FetchAsync_RegistryNotFound_EmitsErrorAndReturnsEmpty() var (errors, _, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("registry"); @@ -83,7 +103,7 @@ public async Task FetchAsync_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty( var (errors, _, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler, maxAttempts: 3); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("2-missing.yaml"); @@ -110,7 +130,7 @@ public async Task FetchAsync_EntryRecoversAfterRetry_ReturnsEntry() var (errors, warnings, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); warnings.Should().BeEmpty(); @@ -126,7 +146,7 @@ public async Task FetchAsync_SchemaVersionTooNew_EmitsError() var (errors, _, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("schema version"); @@ -142,7 +162,7 @@ public async Task FetchAsync_UnsafeFileName_EmitsWarningAndSkips() var (errors, warnings, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); entries.Select(e => e.FileName).Should().BeEquivalentTo("ok.yaml"); From 562bcab978795e9c3fa3d9c4e218216eff2e5ed0 Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Tue, 30 Jun 2026 11:25:16 -0300 Subject: [PATCH 5/6] changelog: align reference docs with {org}/{repo}/{branch} layout Addresses review feedback on the changelog docs that still described the repo-only entry layout. Updates cmd-upload, configure-changelogs-ref, and the changelog-bundle-registry architecture page to changelog/{org}/{repo}/{branch}/... (entries and the per-pool registry.json), documents owner/branch resolution and the new bundle.branch config, and notes uploads can run from any branch. Resolves the open question from review: the entry registry lives at the branch level (changelog/{org}/{repo}/{branch}/registry.json), not repo-level. --- docs/cli/changelog/cmd-upload.md | 21 ++++----- docs/contribute/configure-changelogs-ref.md | 13 +++--- docs/development/changelog-bundle-registry.md | 44 +++++++++++-------- 3 files changed, 43 insertions(+), 35 deletions(-) diff --git a/docs/cli/changelog/cmd-upload.md b/docs/cli/changelog/cmd-upload.md index 2b9be2dc8..26f3f2771 100644 --- a/docs/cli/changelog/cmd-upload.md +++ b/docs/cli/changelog/cmd-upload.md @@ -1,6 +1,6 @@ ## Description -Upload changelog entries or bundle artifacts to S3 or Elasticsearch. The command discovers `.yaml` and `.yml` files in a local directory and uploads only files whose content hash changed since the last run. Changelog entries are uploaded once under `changelog/{repo}/{file}`, keyed by the authoring repository; bundles are uploaded under `bundle/{product}/{file}`, product-scoped from the bundle YAML. +Upload changelog entries or bundle artifacts to S3 or Elasticsearch. The command discovers `.yaml` and `.yml` files in a local directory and uploads only files whose content hash changed since the last run. Changelog entries are uploaded once under `changelog/{org}/{repo}/{branch}/{file}`, keyed by the authoring owner, repository, and branch; bundles are uploaded under `bundle/{product}/{file}`, product-scoped from the bundle YAML. To create bundles first, use [](/cli/changelog/bundle.md). For the end-to-end workflow, see [](/contribute/bundle-changelogs.md). @@ -43,7 +43,7 @@ Your IAM policy must allow these S3 actions on the target bucket: You can scope object-level permissions to the key prefixes the command writes: - `bundle/*` (bundle YAML and `bundle/{product}/registry.json`) -- `changelog/*` (entry YAML and `changelog/{repo}/registry.json`) +- `changelog/*` (entry YAML and `changelog/{org}/{repo}/{branch}/registry.json`) #### Local development @@ -79,7 +79,7 @@ Use `--artifact-type` to choose what to upload: Keying differs by artifact type: -- **Changelog entries** are uploaded **once** under the authoring repository, regardless of how many products they list (or none). The repo is resolved from `--repo`, then `bundle.repo` in `changelog.yml`, then the git remote origin. +- **Changelog entries** are uploaded **once** under the authoring owner/repo/branch, regardless of how many products they list (or none). The owner is resolved from `--owner`, then `bundle.owner` in `changelog.yml`, then the git remote origin; the repo from `--repo`, then `bundle.repo`, then the git remote origin; the branch from `--branch`, then the current checkout's branch. The branch is stored verbatim, so a branch name containing `/` (for example `feature/foo`) becomes additional key segments. - **Bundles** are uploaded once per product listed in the bundle's `products[].product` field (a bundle that declares multiple products is written under each product prefix). ## Upload targets @@ -96,17 +96,17 @@ Use `--target` to choose the destination: For each discovered file, the command writes to: ```text -s3://{bucket}/changelog/{repo}/{filename} # --artifact-type changelog -s3://{bucket}/bundle/{product}/{filename} # --artifact-type bundle +s3://{bucket}/changelog/{org}/{repo}/{branch}/{filename} # --artifact-type changelog +s3://{bucket}/bundle/{product}/{filename} # --artifact-type bundle ``` -Changelog entries are written once under the authoring repo. A bundle that applies to multiple products is uploaded to multiple keys — one per product. +Changelog entries are written once under the authoring org/repo/branch. A bundle that applies to multiple products is uploaded to multiple keys — one per product. After a successful upload, the command refreshes the relevant `registry.json` manifest: ```text -s3://{bucket}/changelog/{repo}/registry.json # changelog uploads -s3://{bucket}/bundle/{product}/registry.json # bundle uploads +s3://{bucket}/changelog/{org}/{repo}/{branch}/registry.json # changelog uploads +s3://{bucket}/bundle/{product}/registry.json # bundle uploads ``` When several repositories publish bundles for the same shared product (for example `cloud-serverless`), use a `{repo}-{dateOrVersion}.yaml` bundle filename convention so they don't overwrite each other under `bundle/{product}/`. @@ -142,15 +142,16 @@ docs-builder changelog upload \ ### Upload changelog entries to S3 -Upload individual changelog YAML files from the default changelog directory (`docs/changelog`). Entries are written to `changelog/{repo}/...`; pass `--repo` (and optionally `--owner`) when the authoring repo can't be inferred from `bundle.repo` or the git remote: +Upload individual changelog YAML files from the default changelog directory (`docs/changelog`). Entries are written to `changelog/{org}/{repo}/{branch}/...`; pass `--owner`, `--repo`, and `--branch` when the authoring owner/repo can't be inferred from `bundle.owner`/`bundle.repo` or the git remote, or to override the current checkout's branch: ```sh docs-builder changelog upload \ --artifact-type changelog \ --target s3 \ --s3-bucket-name my-changelog-bundles \ + --owner elastic \ --repo my-repo \ - --owner elastic + --branch main ``` ### Override the source directory diff --git a/docs/contribute/configure-changelogs-ref.md b/docs/contribute/configure-changelogs-ref.md index b0e36da8f..e2c6015d4 100644 --- a/docs/contribute/configure-changelogs-ref.md +++ b/docs/contribute/configure-changelogs-ref.md @@ -46,12 +46,13 @@ These settings are relevant to one or all of the `changelog bundle`, `changelog | Setting | Description | | ------------------------- | ----------- | +| `bundle.branch` | Branch whose CDN changelog pool (`changelog/{org}/{repo}/{branch}/...`) entries are sourced from when bundling (default: `main`). Refer to [Entry sourcing](#bundle-entry-sourcing). | | `bundle.directory` | Input directory containing changelog YAML files (default: `docs/changelog`). | | `bundle.link_allow_repos` | List of `owner/repo` pairs whose PR/issue links are preserved. When set (including empty `[]`), links to unlisted repos become `# PRIVATE:` sentinels. Requires `bundle.resolve: true` | | `bundle.output_directory` | Output directory for bundled files (default: `docs/releases`). | -| `bundle.owner` | Default GitHub repository owner (for example, `elastic`). | +| `bundle.owner` | Default GitHub repository owner (for example, `elastic`). Also the org segment of uploaded changelog-entry keys (`changelog/{org}/{repo}/{branch}/...`) and CDN entry sourcing. | | `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, and to scope uploaded changelog-entry keys (`changelog/{repo}/...`) and CDN entry sourcing. Only needed when the product ID doesn't match the GitHub repository name (or to override the git remote). | +| `bundle.repo` | Default GitHub repository name (for example, `elasticsearch`). Used by the `{changelog}` directive to generate correct PR and issue links, and to scope uploaded changelog-entry keys (`changelog/{org}/{repo}/{branch}/...`) and CDN entry sourcing. Only needed when the product ID doesn't match the GitHub repository name (or to override the git remote). | | `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). | @@ -66,16 +67,16 @@ When `bundle.link_allow_repos` is omitted, no link filtering occurs. ### Entry sourcing [bundle-entry-sourcing] -`changelog bundle` reads the individual changelog entries it aggregates either from the local folder or from the public CDN. Entries are stored on the CDN per **authoring repository** (`changelog/{repo}/...`), not per product, so CDN sourcing keys off the resolvable authoring repo rather than the bundle's target products. +`changelog bundle` reads the individual changelog entries it aggregates either from the local folder or from the public CDN. Entries are stored on the CDN per **authoring org/repo/branch** (`changelog/{org}/{repo}/{branch}/...`), not per product, so CDN sourcing keys off the resolvable authoring org/repo/branch rather than the bundle's target products. -The authoring repo is resolved with the same precedence as `changelog upload`: `--repo` > `bundle.repo` in `changelog.yml` > the git remote origin. +The authoring repo is resolved with the same precedence as `changelog upload`: `--repo` > `bundle.repo` in `changelog.yml` > the git remote origin. The owner is resolved from `--owner` > `bundle.owner` (default `elastic`), and the branch from `--branch` > `bundle.branch` (default `main`). Sourcing is decided per run: - **Local folder.** Used when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when the authoring repo cannot be resolved. The folder must contain the changelog files. -- **CDN (default when a repo resolves).** Used when the authoring repo resolves, local sourcing is not forced, and a CDN base URL is configured (`DOCS_BUILDER_CHANGELOG_CDN`, defaulting to the public distribution). The command fetches `changelog/{repo}/registry.json` and the entries it lists, then applies the bundle's own product/PR/issue filters to the downloaded set. +- **CDN (default when a repo resolves).** Used when the authoring repo resolves, local sourcing is not forced, and a CDN base URL is configured (`DOCS_BUILDER_CHANGELOG_CDN`, defaulting to the public distribution). The command fetches `changelog/{org}/{repo}/{branch}/registry.json` and the entries it lists, then applies the bundle's own product/PR/issue filters to the downloaded set. -Because entries are repo-scoped, one repository can produce a bundle for a shared product (for example, `cloud-serverless`) while sourcing its own entries from `changelog/{repo}/`, without that product appearing in the repository's `docset.yml`. The `{changelog}` directive's `:cdn:` mode still consumes product-scoped *bundles*, so a repository that also renders its own release notes declares each product under `release_notes` as before. +Because entries are org/repo/branch-scoped, one repository can produce a bundle for a shared product (for example, `cloud-serverless`) while sourcing its own entries from `changelog/{org}/{repo}/{branch}/`, without that product appearing in the repository's `docset.yml`. The `{changelog}` directive's `:cdn:` mode still consumes product-scoped *bundles*, so a repository that also renders its own release notes declares each product under `release_notes` as before. ### Bundle descriptions [bundle-descriptions] diff --git a/docs/development/changelog-bundle-registry.md b/docs/development/changelog-bundle-registry.md index 0bcfc82b0..2f3072715 100644 --- a/docs/development/changelog-bundle-registry.md +++ b/docs/development/changelog-bundle-registry.md @@ -48,8 +48,9 @@ copies, no cross-repo file syncing. docs-actions changelog upload workflow) uploads each bundle to `bundle/{product}/{file}` in the **private** bucket, then refreshes `bundle/{product}/registry.json` for every product the run touched. The companion - `--artifact-type changelog` upload writes individual entries to `changelog/{repo}/{file}` - (one copy, keyed by the authoring repo) and refreshes `changelog/{repo}/registry.json`. + `--artifact-type changelog` upload writes individual entries to `changelog/{org}/{repo}/{branch}/{file}` + (one copy, keyed by the authoring org/repo/branch) and refreshes + `changelog/{org}/{repo}/{branch}/registry.json`. 2. **Scrubber Lambda** — triggered by `s3:ObjectCreated` on the private bucket, it scrubs private repository references out of bundle and entry YAML and writes the sanitized copy to the **public** bucket. The `registry.json` object is copied through **verbatim**. @@ -65,7 +66,7 @@ cacheable manifest at a predictable key that lists exactly which bundles exist f ## `registry.json` format -Stored at `bundle/{product}/registry.json` (bundle index) or `changelog/{repo}/registry.json` +Stored at `bundle/{product}/registry.json` (bundle index) or `changelog/{org}/{repo}/{branch}/registry.json` (changelog-entry index). Serialized with `snake_case` keys. ```json @@ -83,9 +84,9 @@ Stored at `bundle/{product}/registry.json` (bundle index) or `changelog/{repo}/r | Field | Meaning | |---|---| | `schema_version` | Bumped when consumers must change their parser. | -| `product` | Grouping identifier; the second S3 key segment — the product for a bundle index (`bundle/{product}/…`) or the authoring repo for a changelog-entry index (`changelog/{repo}/…`). | +| `product` | Grouping identifier — the product for a bundle index (`bundle/{product}/…`) or the `{org}/{repo}/{branch}` prefix for a changelog-entry index (`changelog/{org}/{repo}/{branch}/…`). | | `generated_at` | UTC timestamp of the last regeneration. | -| `bundles[].file` | Bundle file name, resolved at `bundle/{product}/{file}` (or entry file at `changelog/{repo}/{file}` for the entry index). | +| `bundles[].file` | Bundle file name, resolved at `bundle/{product}/{file}` (or entry file at `changelog/{org}/{repo}/{branch}/{file}` for the entry index). | | `bundles[].target` | Target version/date from the bundle's declaration of **this** product (may be null). | | `bundles[].etag` | See the ETag caveat below. | @@ -108,8 +109,9 @@ reads of the same bucket). The refresh runs inside `ChangelogUploadService` after a successful **bundle** upload (it is skipped for `--artifact-type changelog`). `RegistryBuilder`: -- Groups the run's upload targets by the second key segment — product for bundles - (`bundle/{product}/{file}`), repo for entries (`changelog/{repo}/{file}`). +- Groups the run's upload targets by group key — product for bundles + (`bundle/{product}/{file}`), the `{org}/{repo}/{branch}` prefix for entries + (`changelog/{org}/{repo}/{branch}/{file}`). - For each group, derives one `registry` entry per file (file name, locally-computed S3 ETag, and — for bundle indexes only — that product's target; entry indexes record no target). - Reads the existing manifest from S3, merges by file name (re-uploads replace their entry; @@ -155,8 +157,9 @@ for the producer**: - A CloudFront cache policy tuned for the manifest already exists (default TTL 1h, min 60s). The scrubber only passes through keys accepted by `RegistryKey.IsRegistry` -(`bundle/{product}/registry.json` or `changelog/{repo}/registry.json`, each a single middle -segment), so arbitrary JSON cannot reach the public surface. +(`bundle/{product}/registry.json` with a single product segment, or +`changelog/{org}/{repo}/{branch}/registry.json` with at least three segments), so arbitrary JSON +cannot reach the public surface. **No new docs-actions workflow logic is required** for the producer either: the refresh is a side-effect of the existing `changelog upload` step; docs-actions only needs a docs-builder @@ -171,18 +174,21 @@ build that includes this feature. Consumers must therefore treat a missing bundle as non-fatal (skip + warn), not an error. -## `changelog bundle` entry sourcing (repo gate) +## `changelog bundle` entry sourcing (org/repo/branch gate) The `changelog bundle` command aggregates individual changelog **entries**. It can read those -entries from the local folder or fetch the **authoring repo's** published entries from the CDN -(`changelog/{repo}/registry.json` → `changelog/{repo}/{file}`, via `CdnChangelogEntryFetcher`). - -Under the artifact-root layout, entries are repo-scoped — not product-scoped — so CDN entry -sourcing keys off the resolvable authoring repo (the same precedence as upload: -`--repo` > `bundle.repo` > git remote), **not** off the bundle's target products. This is what lets -one repo (for example `kibana`) produce a bundle for a shared product (for example `cloud-serverless`) -while sourcing its own entries from `changelog/kibana/`, without that product appearing in the repo's -own `docset.yml`. The decision is made per run by `ChangelogBundlingService`: +entries from the local folder or fetch the **authoring pool's** published entries from the CDN +(`changelog/{org}/{repo}/{branch}/registry.json` → `changelog/{org}/{repo}/{branch}/{file}`, via +`CdnChangelogEntryFetcher`). + +Under the artifact-root layout, entries are org/repo/branch-scoped — not product-scoped — so CDN +entry sourcing keys off the resolvable authoring pool (repo with the same precedence as upload: +`--repo` > `bundle.repo` > git remote; owner from `--owner` > `bundle.owner`, default `elastic`; +branch from `--branch` > `bundle.branch`, default `main`), **not** off the bundle's target products. +This is what lets one repo (for example `kibana`) produce a bundle for a shared product (for example +`cloud-serverless`) while sourcing its own entries from `changelog/elastic/kibana/main/`, without that +product appearing in the repo's own `docset.yml`. The decision is made per run by +`ChangelogBundlingService`: - **Local folder** when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when the authoring repo cannot be resolved. From a8aeb9fc5828a39489579069fee1db422afb794b Mon Sep 17 00:00:00 2001 From: Felipe Cotti Date: Tue, 30 Jun 2026 11:40:55 -0300 Subject: [PATCH 6/6] changelog: harden CDN entry pool resolution (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit findings on the consumer-side org/repo/branch handling: - CdnChangelogEntryFetcher now validates the org, repo, and each branch segment before building the CDN URI, rejecting empty/"."/".." segments so a malformed branch from config/CLI cannot normalize "../" into a different pool. - ChangelogBundlingService derives the org from a combined owner/repo value (e.g. acme/widget -> acme) when no explicit owner is set, instead of dropping the prefix and defaulting to "elastic" — keeping the CDN pool path correct. Skipped the suggestion to strip "+" in NormalizeRepo: bundle.repo with "+" merged-repo syntax is already rejected at config load (ParseBundleConfiguration). Adds tests for the unsafe-branch rejection and owner-from-combined-repo path. --- .../ReleaseNotes/CdnChangelogEntryFetcher.cs | 37 ++++++++++++++++++- .../Bundling/ChangelogBundlingService.cs | 22 ++++++++++- .../Changelogs/BundleCdnSourcingTests.cs | 24 ++++++++++++ .../CdnChangelogEntryFetcherTests.cs | 19 ++++++++++ 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs index 757002196..a11dc6281 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs @@ -106,8 +106,19 @@ public async Task> FetchAsync( Action emitWarning, Cancel ctx) { - var poolSegments = PoolSegments(org, repo, branch).ToArray(); var poolLabel = $"{org}/{repo}/{branch}"; + + // Defense-in-depth: org/repo come from config and branch from config or CLI on the consumer side. + // Reject empty or traversal segments before building the URI so it cannot normalize a "../" into a + // different changelog pool than intended. + if (!IsValidPool(org, repo, branch)) + { + emitError( + $"Invalid changelog pool '{poolLabel}': the org, repo, and each branch segment must be non-empty, contain no path separators, and not be '.' or '..'."); + return []; + } + + var poolSegments = PoolSegments(org, repo, branch).ToArray(); var registryUri = CombineSegments(baseUri, [.. poolSegments, "registry.json"]); ChangelogRegistry? registry; @@ -266,6 +277,30 @@ private static IEnumerable PoolSegments(string org, string repo, string yield return part; } + /// + /// Validates the consumer-supplied pool coordinates (org, repo, and each /-delimited branch + /// segment) so a malformed branch cannot redirect the fetch to a different pool via URI normalization. + /// + private static bool IsValidPool(string org, string repo, string branch) + { + if (!IsSafePoolSegment(org) || !IsSafePoolSegment(repo)) + return false; + + foreach (var part in branch.Split('/')) + { + if (!IsSafePoolSegment(part)) + return false; + } + + return true; + } + + private static bool IsSafePoolSegment(string segment) => + !string.IsNullOrEmpty(segment) + && segment is not ("." or "..") + && !segment.Contains('/', StringComparison.Ordinal) + && !segment.Contains('\\', StringComparison.Ordinal); + /// /// 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 changelog/{org}/{repo}/{branch}/{file}). diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index 96156401d..e31d7f278 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -227,7 +227,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // configured. This stays in lockstep with PlanBundleAsync's needs_network decision. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var authoringRepo = NormalizeRepo(input.Repo); - var authoringOwner = string.IsNullOrWhiteSpace(input.Owner) ? DefaultOwner : input.Owner; + var authoringOwner = ResolveAuthoringOwner(input.Owner, input.Repo); var authoringBranch = string.IsNullOrWhiteSpace(input.Branch) ? DefaultBranch : input.Branch; var useCdn = ShouldSourceFromCdn(authoringRepo, useLocalChangelogs: useLocalChangelogs, explicitDirectory: explicitDirectory); @@ -811,6 +811,26 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return slash >= 0 && slash < repo.Length - 1 ? repo[(slash + 1)..] : repo; } + /// + /// Resolves the org segment for the CDN entry pool: the explicit when set, + /// otherwise the owner/ prefix of a combined value (e.g. acme/widget + /// -> acme) so it is not lost, falling back to . + /// + private static string ResolveAuthoringOwner(string? owner, string? repo) + { + if (!string.IsNullOrWhiteSpace(owner)) + return owner; + + if (!string.IsNullOrWhiteSpace(repo)) + { + var slash = repo.IndexOf('/', StringComparison.Ordinal); + if (slash > 0) + return repo[..slash]; + } + + return DefaultOwner; + } + /// Gate for repo-scoped CDN entry sourcing: true when the authoring repo resolves, local sourcing is not forced (bundle.use_local_changelogs/--directory), and a CDN base is configured. private static bool ShouldSourceFromCdn(string? authoringRepo, bool useLocalChangelogs, bool explicitDirectory) { diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs index 73a59e4c5..ce48121a9 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs @@ -124,6 +124,30 @@ public async Task OptionMode_OwnerAndBranchOverride_SourcesFromThatPoolOnCdn() handler.RequestedPaths.Should().Contain("/changelog/acme-corp/elasticsearch/8.x/registry.json"); } + [Fact] + public async Task OptionMode_OwnerFromCombinedRepo_SourcesFromThatPool() + { + // When --repo is given in owner/repo form and no explicit owner is set, the owner segment must be + // taken from the repo prefix (not defaulted to elastic), so the CDN pool path stays correct. + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher(Output, handler)); + var output = OutputPath(); + + var input = new BundleChangelogsArguments + { + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = output, + Repo = "acme-corp/widget", + 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); + handler.RequestedPaths.Should().Contain("/changelog/acme-corp/widget/main/registry.json"); + } + [Fact] public async Task OptionMode_NoResolvableRepo_FallsBackToLocal() { diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs index e1e9f7061..7d2f18278 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs @@ -74,6 +74,25 @@ public async Task FetchAsync_BranchWithSlashes_KeepsBranchSeparatorsInPath() handler.RequestedPaths.Should().Contain(p => p.EndsWith("/changelog/elastic/elasticsearch/feature/foo/1-a.yaml", StringComparison.Ordinal)); } + [Theory] + [InlineData("..")] + [InlineData("feature/..")] + [InlineData("")] + public async Task FetchAsync_UnsafeBranch_EmitsErrorAndDoesNotHitCdn(string branch) + { + // A traversal/empty branch segment must be rejected before any request, so URI normalization + // can't redirect the fetch to a different pool. + var handler = new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.OK)); + var (errors, _, emitError, emitWarning) = Diagnostics(); + + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", branch, emitError, emitWarning, TestContext.Current.CancellationToken); + + entries.Should().BeEmpty(); + errors.Should().ContainSingle().Which.Should().Contain("Invalid changelog pool"); + handler.RequestedPaths.Should().BeEmpty("validation must happen before any CDN request"); + } + [Fact] public async Task FetchAsync_RegistryNotFound_EmitsErrorAndReturnsEmpty() {